Messages in this thread | ![/](/images/icornerl.gif) | | Date | Fri, 17 May 2024 00:43:31 +0900 (JST) | Subject | Re: [PATCH net] nfc: nci: Fix handling of zero-length payload packets in nci_rx_work() | From | Shigeru Yoshida <> |
| |
Hi Yasuoka-san,
On Thu, 16 May 2024 18:24:54 +0900, Ryosuke Yasuoka wrote: > Thank you for your review and comment, Simon. > > On Thu, May 16, 2024 at 09:43:48AM +0100, Simon Horman wrote: >> Hi Yasuoka-san, >> >> On Thu, May 16, 2024 at 12:17:07AM +0900, Ryosuke Yasuoka wrote: >> > When nci_rx_work() receives a zero-length payload packet, it should >> > discard the packet without exiting the loop. Instead, it should continue >> > processing subsequent packets. >> >> nit: I think it would be clearer to say: >> >> ... it should not discard the packet and exit the loop. Instead, ... > > Great. I'll update commit msg like this. > >> > >> > Fixes: d24b03535e5e ("nfc: nci: Fix uninit-value in nci_dev_up and nci_ntf_packet") >> > Closes: https://lore.kernel.org/lkml/20240428134525.GW516117@kernel.org/T/ >> >> nit: I'm not sure this Closes link is adding much, >> there are more changes coming, right? > > No. I just wanna show the URL link as a reference where this bug is > found. This URL discuss a little bit different topic as you know. > > In the following discussion [1], Jakub pointed out that changing > continue statement to break is not related to the patch "Fix > uninit-value in nci_rw_work". So I posted this new small patch before > posting v5 patch for "Fix: uninit-value in nci_rw_work". > > If Closes tag is not appropriate, I can remove this in this v2 patch. > What do you think?
I think this patch, continuing the loop after freeing skb, came from the following discussion:
https://lore.kernel.org/lkml/Zi-vGH1ROjiv1yJ2@zeus/
In such a case, you can use Link: tag with the above URL and mention it in the change log like "As discussed [1], ...". But this patch small and clear, so I think the current change log is enough to convey the intent of the changes without external link.
You may find the following document helpful:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v6.9#n115
Thanks, Shigeru
> > [1] https://lore.kernel.org/all/20240510190613.72838bf0@kernel.org/ > >> > Reported-by: Ryosuke Yasuoka <ryasuoka@redhat.com> >> > Signed-off-by: Ryosuke Yasuoka <ryasuoka@redhat.com> >> > --- >> > net/nfc/nci/core.c | 3 +-- >> > 1 file changed, 1 insertion(+), 2 deletions(-) >> > >> > diff --git a/net/nfc/nci/core.c b/net/nfc/nci/core.c >> > index b133dc55304c..f2ae8b0d81b9 100644 >> > --- a/net/nfc/nci/core.c >> > +++ b/net/nfc/nci/core.c >> > @@ -1518,8 +1518,7 @@ static void nci_rx_work(struct work_struct *work) >> > >> > if (!nci_plen(skb->data)) { >> > kfree_skb(skb); >> > - kcov_remote_stop(); >> > - break; >> > + continue; >> > } >> > >> > /* Process frame */ >> > -- >> > 2.44.0 >> > >> > >> > > Thank you for your help. > Ryosuke > >
| ![\](/images/icornerr.gif) |