linux-kernel - Re: [PATCH] um: vector: fix return value check in vector_legacy

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1c098c26132b60ebebc46a39e8b7827a29e3a166.camel@sipsolutions.net>
Date: Fri, 05 Jan 2024 09:42:12 +0100
From: Johannes Berg <johannes@...solutions.net>
To: Richard Weinberger <richard.weinberger@...il.com>, Ma Ke
	 <make_ruc2021@....com>
Cc: richard@....at, anton.ivanov@...bridgegreys.com, xiangyang3@...wei.com, 
	linux-um@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] um: vector: fix return value check in vector_legacy_rx

On Thu, 2024-01-04 at 22:05 +0100, Richard Weinberger wrote:
> On Fri, Oct 6, 2023 at 2:28 PM Ma Ke <make_ruc2021@....com> wrote:
> > 
> > In vector_legacy_rx, to avoid an unexpected result returned by
> > pskb_trim, we should check the return value of pskb_trim().
> > 
> > Signed-off-by: Ma Ke <make_ruc2021@....com>
> > ---
> >  arch/um/drivers/vector_kern.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/arch/um/drivers/vector_kern.c b/arch/um/drivers/vector_kern.c
> > index 131b7cb29576..822a8c0cdcc1 100644
> > --- a/arch/um/drivers/vector_kern.c
> > +++ b/arch/um/drivers/vector_kern.c
> > @@ -890,7 +890,8 @@ static int vector_legacy_rx(struct vector_private *vp)
> >                                         skb->ip_summed = CHECKSUM_UNNECESSARY;
> >                                 }
> >                         }
> > -                       pskb_trim(skb, pkt_len - vp->rx_header_size);
> > +                       if (pskb_trim(skb, pkt_len - vp->rx_header_size))
> > +                               return 0;
> 
> I think this adds a memory leak. Also, can pskb_trim() really fail in
> this scenario?
> The function controls skb creation and knows all lengths.
> 
We had pretty much the exact same discussion in the other patch ...

https://patchwork.ozlabs.org/project/linux-um/patch/20231007005104.3994678-1-make_ruc2021@163.com/


No point arguing with people who care about static checkers only, I
guess. This person here never even came back to respond to the comments,
my take is they're throwing patches over the wall they didn't think
about, just to see what sticks.

johannes