All of lore.kernel.org
 help / color / mirror / Atom feed
From: Manivannan Sadhasivam <mani@kernel.org>
To: Pavel Skripkin <paskripkin@gmail.com>
Cc: davem@davemloft.net, kuba@kernel.org,
	bjorn.andersson@sonymobile.com, courtney.cavin@sonymobile.com,
	linux-arm-msm@vger.kernel.org, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org,
	syzbot+35a511c72ea7356cdcf3@syzkaller.appspotmail.com
Subject: Re: [PATCH] net: qrtr: fix memory leak in qrtr_local_enqueue
Date: Fri, 23 Jul 2021 17:57:53 +0530	[thread overview]
Message-ID: <20210723122753.GA3739@thinkpad> (raw)
In-Reply-To: <20210722161625.6956-1-paskripkin@gmail.com>

On Thu, Jul 22, 2021 at 07:16:25PM +0300, Pavel Skripkin wrote:
> Syzbot reported memory leak in qrtr. The problem was in unputted
> struct sock. qrtr_local_enqueue() function calls qrtr_port_lookup()
> which takes sock reference if port was found. Then there is the following
> check:
> 
> if (!ipc || &ipc->sk == skb->sk) {
> 	...
> 	return -ENODEV;
> }
> 
> Since we should drop the reference before returning from this function and
> ipc can be non-NULL inside this if, we should add qrtr_port_put() inside
> this if.
> 
> Fixes: bdabad3e363d ("net: Add Qualcomm IPC router")
> Reported-and-tested-by: syzbot+35a511c72ea7356cdcf3@syzkaller.appspotmail.com
> Signed-off-by: Pavel Skripkin <paskripkin@gmail.com>

Reviewed-by: Manivannan Sadhasivam <mani@kernel.org>

It'd be good if this patch can be extended to fix one more corner case here:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/net/qrtr/qrtr.c#n522

Thanks,
Mani

> ---
>  net/qrtr/qrtr.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/net/qrtr/qrtr.c b/net/qrtr/qrtr.c
> index e6f4a6202f82..d5ce428d0b25 100644
> --- a/net/qrtr/qrtr.c
> +++ b/net/qrtr/qrtr.c
> @@ -839,6 +839,8 @@ static int qrtr_local_enqueue(struct qrtr_node *node, struct sk_buff *skb,
>  
>  	ipc = qrtr_port_lookup(to->sq_port);
>  	if (!ipc || &ipc->sk == skb->sk) { /* do not send to self */
> +		if (ipc)
> +			qrtr_port_put(ipc);
>  		kfree_skb(skb);
>  		return -ENODEV;
>  	}
> -- 
> 2.32.0
> 

  reply	other threads:[~2021-07-23 12:28 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-22 16:16 [PATCH] net: qrtr: fix memory leak in qrtr_local_enqueue Pavel Skripkin
2021-07-23 12:27 ` Manivannan Sadhasivam [this message]
2021-07-23 15:08   ` Pavel Skripkin
2021-07-23 15:31   ` [PATCH v2] net: qrtr: fix memory leaks Pavel Skripkin
2021-07-23 15:43     ` Manivannan Sadhasivam
2021-07-23 16:50     ` patchwork-bot+netdevbpf

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210723122753.GA3739@thinkpad \
    --to=mani@kernel.org \
    --cc=bjorn.andersson@sonymobile.com \
    --cc=courtney.cavin@sonymobile.com \
    --cc=davem@davemloft.net \
    --cc=kuba@kernel.org \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=paskripkin@gmail.com \
    --cc=syzbot+35a511c72ea7356cdcf3@syzkaller.appspotmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.