| Message ID | 20231206111329.GA9888@ubuntu (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [v3] net/rose: Fix Use-After-Free in rose_ioctl | expand |
On Wed, Dec 6, 2023 at 12:13 PM Hyunwoo Kim <v4bel@theori.io> wrote: > > --- > net/rose/af_rose.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/net/rose/af_rose.c b/net/rose/af_rose.c > index 0cc5a4e19900..7ff76bf3f56e 100644 > --- a/net/rose/af_rose.c > +++ b/net/rose/af_rose.c > @@ -1316,8 +1316,10 @@ static int rose_ioctl(struct socket *sock, unsigned int cmd, unsigned long arg) > struct sk_buff *skb; > long amount = 0L; > /* These two are safe on a single CPU system as only user tasks fiddle here */ Can you remove this stale and confused comment ? Thanks.
diff --git a/net/rose/af_rose.c b/net/rose/af_rose.c index 0cc5a4e19900..7ff76bf3f56e 100644 --- a/net/rose/af_rose.c +++ b/net/rose/af_rose.c @@ -1316,8 +1316,10 @@ static int rose_ioctl(struct socket *sock, unsigned int cmd, unsigned long arg) struct sk_buff *skb; long amount = 0L; /* These two are safe on a single CPU system as only user tasks fiddle here */ + spin_lock_irq(&sk->sk_receive_queue.lock); if ((skb = skb_peek(&sk->sk_receive_queue)) != NULL) amount = skb->len; + spin_unlock_irq(&sk->sk_receive_queue.lock); return put_user(amount, (unsigned int __user *) argp); }
Because rose_ioctl() accesses sk->sk_receive_queue without holding a sk->sk_receive_queue.lock, it can cause a race with rose_accept(). A use-after-free for skb occurs with the following flow. ``` rose_ioctl() -> skb_peek() rose_accept() -> skb_dequeue() -> kfree_skb() ``` Add sk->sk_receive_queue.lock to rose_ioctl() to fix this issue. Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2") Signed-off-by: Hyunwoo Kim <v4bel@theori.io> --- v1 -> v2: Use sk->sk_receive_queue.lock instead of lock_sock. v2 -> v3: Change spin_lock to spin_lock_irq --- net/rose/af_rose.c | 2 ++ 1 file changed, 2 insertions(+)