* [PATCH] um: vector: fix return value check in vector_legacy_rx @ 2023-10-06 12:27 Ma Ke 2024-01-04 21:05 ` Richard Weinberger 0 siblings, 1 reply; 5+ messages in thread From: Ma Ke @ 2023-10-06 12:27 UTC (permalink / raw) To: richard, anton.ivanov, johannes, make_ruc2021, xiangyang3 Cc: linux-um, linux-kernel 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@163.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; skb->protocol = eth_type_trans(skb, skb->dev); vp->dev->stats.rx_bytes += skb->len; vp->dev->stats.rx_packets++; -- 2.37.2 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] um: vector: fix return value check in vector_legacy_rx 2023-10-06 12:27 [PATCH] um: vector: fix return value check in vector_legacy_rx Ma Ke @ 2024-01-04 21:05 ` Richard Weinberger 2024-01-05 8:42 ` Johannes Berg 0 siblings, 1 reply; 5+ messages in thread From: Richard Weinberger @ 2024-01-04 21:05 UTC (permalink / raw) To: Ma Ke; +Cc: richard, anton.ivanov, johannes, xiangyang3, linux-um, linux-kernel On Fri, Oct 6, 2023 at 2:28 PM Ma Ke <make_ruc2021@163.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@163.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. ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] um: vector: fix return value check in vector_legacy_rx 2024-01-04 21:05 ` Richard Weinberger @ 2024-01-05 8:42 ` Johannes Berg 2024-01-05 8:52 ` Richard Weinberger 2024-01-05 11:23 ` Anton Ivanov 0 siblings, 2 replies; 5+ messages in thread From: Johannes Berg @ 2024-01-05 8:42 UTC (permalink / raw) To: Richard Weinberger, Ma Ke Cc: richard, anton.ivanov, xiangyang3, linux-um, linux-kernel 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@163.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@163.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 ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] um: vector: fix return value check in vector_legacy_rx 2024-01-05 8:42 ` Johannes Berg @ 2024-01-05 8:52 ` Richard Weinberger 2024-01-05 11:23 ` Anton Ivanov 1 sibling, 0 replies; 5+ messages in thread From: Richard Weinberger @ 2024-01-05 8:52 UTC (permalink / raw) To: Johannes Berg; +Cc: Ma Ke, anton ivanov, xiangyang3, linux-um, linux-kernel ----- Ursprüngliche Mail ----- > Von: "Johannes Berg" <johannes@sipsolutions.net> > An: "Richard Weinberger" <richard.weinberger@gmail.com>, "Ma Ke" <make_ruc2021@163.com> > CC: "richard" <richard@nod.at>, "anton ivanov" <anton.ivanov@cambridgegreys.com>, xiangyang3@huawei.com, "linux-um" > <linux-um@lists.infradead.org>, "linux-kernel" <linux-kernel@vger.kernel.org> > Gesendet: Freitag, 5. Januar 2024 09:42:12 > Betreff: 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@163.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@163.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/ Yeah, I saw that discussion after commenting to this patch. (My fault, I scanned patchwork bottom up) > > 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. Drive-by fixes are often a waste of everyone's time. :-( Thanks, //richard ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] um: vector: fix return value check in vector_legacy_rx 2024-01-05 8:42 ` Johannes Berg 2024-01-05 8:52 ` Richard Weinberger @ 2024-01-05 11:23 ` Anton Ivanov 1 sibling, 0 replies; 5+ messages in thread From: Anton Ivanov @ 2024-01-05 11:23 UTC (permalink / raw) To: Johannes Berg, Richard Weinberger, Ma Ke Cc: richard, xiangyang3, linux-um, linux-kernel On 05/01/2024 08:42, Johannes Berg wrote: > 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@163.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@163.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. CV padding, the 2024 way. Accomplishments: submitted 231 patches to the linux kernel. > > johannes > > -- Anton R. Ivanov Cambridgegreys Limited. Registered in England. Company Number 10273661 https://www.cambridgegreys.com/ ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2024-01-05 11:24 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-10-06 12:27 [PATCH] um: vector: fix return value check in vector_legacy_rx Ma Ke 2024-01-04 21:05 ` Richard Weinberger 2024-01-05 8:42 ` Johannes Berg 2024-01-05 8:52 ` Richard Weinberger 2024-01-05 11:23 ` Anton Ivanov
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox