lkml.org 
[lkml]   [2019]   [Sep]   [26]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] net: qrtr: fix memory leak in qrtr_tun_read_iter
On Thu, Sep 26, 2019 at 12:21:12AM +0100, Al Viro wrote:
> On Wed, Sep 25, 2019 at 06:04:13PM -0500, Navid Emamdoost wrote:
> > In qrtr_tun_read_iter we need an error handling path to appropriately
> > release skb in cases of error.
>
> Release _what_ skb?
It is not a leak clearly! My bad ...
>
> > Signed-off-by: Navid Emamdoost <navid.emamdoost@gmail.com>
> > ---
> > net/qrtr/tun.c | 13 +++++++++----
> > 1 file changed, 9 insertions(+), 4 deletions(-)
> >
> > diff --git a/net/qrtr/tun.c b/net/qrtr/tun.c
> > index e35869e81766..0f6e6d1d2901 100644
> > --- a/net/qrtr/tun.c
> > +++ b/net/qrtr/tun.c
> > @@ -54,19 +54,24 @@ static ssize_t qrtr_tun_read_iter(struct kiocb *iocb, struct iov_iter *to)
> > int count;
> >
> > while (!(skb = skb_dequeue(&tun->queue))) {
>
> The body of the loop is entered only if the loop condition has
> evaluated true. In this case, it means that the value of
> !(skb = skb_dequeue(&tun->queue))
> had been true, i.e. the value of
> skb = skb_dequeue(&tun->queue)
> has been NULL, i.e. that skb_dequeue() has returned NULL, which had
> been copied into skb.
>
> In other words, in the body of that loop we have skb equal to NULL.
>
> > - if (filp->f_flags & O_NONBLOCK)
> > - return -EAGAIN;
> > + if (filp->f_flags & O_NONBLOCK) {
> > + count = -EAGAIN;
> > + goto out;
> > + }
> >
> > /* Wait until we get data or the endpoint goes away */
> > if (wait_event_interruptible(tun->readq,
> > - !skb_queue_empty(&tun->queue)))
> > - return -ERESTARTSYS;
> > + !skb_queue_empty(&tun->queue))) {
> > + count = -ERESTARTSYS;
> > + goto out;
> > + }
> > }
>
> The meaning of that loop is fairly clear, isn't it? Keep looking int
> tun->queue until an skb shows up there. If it's not immediately there,
> fail with -EAGAIN for non-blocking files and wait on tun->readq until
> some skb arrives.

Thanks for the explainations.

Navid.

\
 
 \ /
  Last update: 2019-09-27 05:16    [W:0.053 / U:0.104 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site