Commit dc34ebd5 authored by Gavrilov Ilia's avatar Gavrilov Ilia Committed by Jakub Kicinski

pppoe: Fix memory leak in pppoe_sendmsg()

syzbot reports a memory leak in pppoe_sendmsg [1].

The problem is in the pppoe_recvmsg() function that handles errors
in the wrong order. For the skb_recv_datagram() function, check
the pointer to skb for NULL first, and then check the 'error' variable,
because the skb_recv_datagram() function can set 'error'
to -EAGAIN in a loop but return a correct pointer to socket buffer
after a number of attempts, though 'error' remains set to -EAGAIN.

skb_recv_datagram
      __skb_recv_datagram          // Loop. if (err == -EAGAIN) then
                                   // go to the next loop iteration
          __skb_try_recv_datagram  // if (skb != NULL) then return 'skb'
                                   // else if a signal is received then
                                   // return -EAGAIN

Found by InfoTeCS on behalf of Linux Verification Center
(linuxtesting.org) with Syzkaller.

Link: https://syzkaller.appspot.com/bug?extid=6bdfd184eac7709e5cc9 [1]

Fixes: 1da177e4 ("Linux-2.6.12-rc2")
Reported-by: syzbot+6bdfd184eac7709e5cc9@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=6bdfd184eac7709e5cc9Signed-off-by: default avatarGavrilov Ilia <Ilia.Gavrilov@infotecs.ru>
Reviewed-by: default avatarGuillaume Nault <gnault@redhat.com>
Link: https://lore.kernel.org/r/20240214085814.3894917-1-Ilia.Gavrilov@infotecs.ruSigned-off-by: default avatarJakub Kicinski <kuba@kernel.org>
parent 4e45170d
...@@ -1007,26 +1007,21 @@ static int pppoe_recvmsg(struct socket *sock, struct msghdr *m, ...@@ -1007,26 +1007,21 @@ static int pppoe_recvmsg(struct socket *sock, struct msghdr *m,
struct sk_buff *skb; struct sk_buff *skb;
int error = 0; int error = 0;
if (sk->sk_state & PPPOX_BOUND) { if (sk->sk_state & PPPOX_BOUND)
error = -EIO; return -EIO;
goto end;
}
skb = skb_recv_datagram(sk, flags, &error); skb = skb_recv_datagram(sk, flags, &error);
if (error < 0) if (!skb)
goto end; return error;
if (skb) { total_len = min_t(size_t, total_len, skb->len);
total_len = min_t(size_t, total_len, skb->len); error = skb_copy_datagram_msg(skb, 0, m, total_len);
error = skb_copy_datagram_msg(skb, 0, m, total_len); if (error == 0) {
if (error == 0) { consume_skb(skb);
consume_skb(skb); return total_len;
return total_len;
}
} }
kfree_skb(skb); kfree_skb(skb);
end:
return error; return error;
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment