| Message ID | 20240210125054.71391-1-remi@remlab.net (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [1/2] phonet: take correct lock to peek at the RX queue | expand |
On Sat, Feb 10, 2024 at 1:50 PM Rémi Denis-Courmont <remi@remlab.net> wrote: > > From: Rémi Denis-Courmont <courmisch@gmail.com> > > Reported-by: Luosili <rootlab@huawei.com> > Signed-off-by: Rémi Denis-Courmont <courmisch@gmail.com> > --- > net/phonet/datagram.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > Fixes: 107d0d9b8d9a ("Phonet: Phonet datagram transport protocol") Reviewed-by: Eric Dumazet <edumazet@google.com> Thanks.
On Sat, 2024-02-10 at 14:50 +0200, Rémi Denis-Courmont wrote: > From: Rémi Denis-Courmont <courmisch@gmail.com> > > Reported-by: Luosili <rootlab@huawei.com> > Signed-off-by: Rémi Denis-Courmont <courmisch@gmail.com> Looks good, but you need to add a non empty commit message. Even something quite similar to the one included in patch 2/2. you can retain Eric's ack when post v2. Pleas also include the correct fixes tag. Thanks Paolo
Hi, Le 13 février 2024 14:12:57 GMT+02:00, Paolo Abeni <pabeni@redhat.com> a écrit : >On Sat, 2024-02-10 at 14:50 +0200, Rémi Denis-Courmont wrote: >> From: Rémi Denis-Courmont <courmisch@gmail.com> >> >> Reported-by: Luosili <rootlab@huawei.com> >> Signed-off-by: Rémi Denis-Courmont <courmisch@gmail.com> > >Looks good, but you need to add a non empty commit message. With all due respect, the headline is self-explanatory in my opinion. You can't compare this with the more involved second patch. Also the second patch was *not* reported by Huawei Rootlab, but inferred by me and thus has no existing documentation - unlike this one. As for the bug ID, I don't know it (security list didn't pass it on to me). Anyhow it seems that Eric Dumazet already either found it or filled it in, so I don't know what else you're asking for.
On Tue, Feb 13, 2024 at 1:55 PM Rémi Denis-Courmont <remi@remlab.net> wrote: > > Hi, > > Le 13 février 2024 14:12:57 GMT+02:00, Paolo Abeni <pabeni@redhat.com> a écrit : > >On Sat, 2024-02-10 at 14:50 +0200, Rémi Denis-Courmont wrote: > >> From: Rémi Denis-Courmont <courmisch@gmail.com> > >> > >> Reported-by: Luosili <rootlab@huawei.com> > >> Signed-off-by: Rémi Denis-Courmont <courmisch@gmail.com> > > > >Looks good, but you need to add a non empty commit message. > > With all due respect, the headline is self-explanatory in my opinion. You can't compare this with the more involved second patch. Also the second patch was *not* reported by Huawei Rootlab, but inferred by me and thus has no existing documentation - unlike this one. > > As for the bug ID, I don't know it (security list didn't pass it on to me). Anyhow it seems that Eric Dumazet already either found it or filled it in, so I don't know what else you're asking for. I do not think patchwork is able to add the "Fixes: " tag that I 'added' And yes, I missed that the changelog was empty. Some words would be nice, even if the patch looks obvious to few of us.
On Tue, 2024-02-13 at 14:55 +0200, Rémi Denis-Courmont wrote: > Le 13 février 2024 14:12:57 GMT+02:00, Paolo Abeni <pabeni@redhat.com> a écrit : > > On Sat, 2024-02-10 at 14:50 +0200, Rémi Denis-Courmont wrote: > > > From: Rémi Denis-Courmont <courmisch@gmail.com> > > > > > > Reported-by: Luosili <rootlab@huawei.com> > > > Signed-off-by: Rémi Denis-Courmont <courmisch@gmail.com> > > > > Looks good, but you need to add a non empty commit message. > > With all due respect, the headline is self-explanatory in my opinion. You can't compare this with the more involved second patch. Also the second patch was *not* reported by Huawei Rootlab, but inferred by me and thus has no existing documentation - unlike this one. > > As for the bug ID, I don't know it (security list didn't pass > it on to me). Anyhow it seems that Eric Dumazet already > either found it or filled it in, so I don't know what else > you're asking for. We don't need a bug ID. We need the hash of the commit introducing the issue. You should be able to find it digging in the git history, looking for the changeset introducing the problematic code. We need such tag for both patches. Eric already provided it for 1/2, you should find it for patch 2/2. See: https://elixir.bootlin.com/linux/latest/source/Documentation/process/5.Posting.rst#L204 Cheers, Paolo
diff --git a/net/phonet/datagram.c b/net/phonet/datagram.c index 3aa50dc7535b..976fe250b509 100644 --- a/net/phonet/datagram.c +++ b/net/phonet/datagram.c @@ -34,10 +34,10 @@ static int pn_ioctl(struct sock *sk, int cmd, int *karg) switch (cmd) { case SIOCINQ: - lock_sock(sk); + spin_lock_bh(&sk->sk_receive_queue.lock); skb = skb_peek(&sk->sk_receive_queue); *karg = skb ? skb->len : 0; - release_sock(sk); + spin_unlock_bh(&sk->sk_receive_queue.lock); return 0; case SIOCPNADDRESOURCE: