* [net PATCH] bpf: skmsg, fix potential psock NULL pointer dereference
@ 2019-11-21 16:25 John Fastabend
2019-11-21 20:15 ` Alexei Starovoitov
0 siblings, 1 reply; 4+ messages in thread
From: John Fastabend @ 2019-11-21 16:25 UTC (permalink / raw
To: davem; +Cc: netdev, john.fastabend, dan.carpenter, daniel
Report from Dan Carpenter,
net/core/skmsg.c:792 sk_psock_write_space()
error: we previously assumed 'psock' could be null (see line 790)
net/core/skmsg.c
789 psock = sk_psock(sk);
790 if (likely(psock && sk_psock_test_state(psock, SK_PSOCK_TX_ENABLED)))
Check for NULL
791 schedule_work(&psock->work);
792 write_space = psock->saved_write_space;
^^^^^^^^^^^^^^^^^^^^^^^^
793 rcu_read_unlock();
794 write_space(sk);
Ensure psock dereference on line 792 only occurs if psock is not null.
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Fixes: 604326b41a6f ("bpf, sockmap: convert to generic sk_msg interface")
Signed-off-by: John Fastabend <john.fastabend@gmail.com>
---
net/core/skmsg.c | 13 ++++++++-----
1 file changed, 8 insertions(+), 5 deletions(-)
diff --git a/net/core/skmsg.c b/net/core/skmsg.c
index ad31e4e..a469d21 100644
--- a/net/core/skmsg.c
+++ b/net/core/skmsg.c
@@ -793,15 +793,18 @@ static void sk_psock_strp_data_ready(struct sock *sk)
static void sk_psock_write_space(struct sock *sk)
{
struct sk_psock *psock;
- void (*write_space)(struct sock *sk);
+ void (*write_space)(struct sock *sk) = NULL;
rcu_read_lock();
psock = sk_psock(sk);
- if (likely(psock && sk_psock_test_state(psock, SK_PSOCK_TX_ENABLED)))
- schedule_work(&psock->work);
- write_space = psock->saved_write_space;
+ if (likely(psock)) {
+ if (sk_psock_test_state(psock, SK_PSOCK_TX_ENABLED))
+ schedule_work(&psock->work);
+ write_space = psock->saved_write_space;
+ }
rcu_read_unlock();
- write_space(sk);
+ if (write_space)
+ write_space(sk);
}
int sk_psock_init_strp(struct sock *sk, struct sk_psock *psock)
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [net PATCH] bpf: skmsg, fix potential psock NULL pointer dereference
2019-11-21 16:25 [net PATCH] bpf: skmsg, fix potential psock NULL pointer dereference John Fastabend
@ 2019-11-21 20:15 ` Alexei Starovoitov
2019-11-21 20:27 ` John Fastabend
0 siblings, 1 reply; 4+ messages in thread
From: Alexei Starovoitov @ 2019-11-21 20:15 UTC (permalink / raw
To: John Fastabend
Cc: David S. Miller, Network Development, Dan Carpenter,
Daniel Borkmann
On Thu, Nov 21, 2019 at 8:28 AM John Fastabend <john.fastabend@gmail.com> wrote:
>
> Report from Dan Carpenter,
>
> net/core/skmsg.c:792 sk_psock_write_space()
> error: we previously assumed 'psock' could be null (see line 790)
>
> net/core/skmsg.c
> 789 psock = sk_psock(sk);
> 790 if (likely(psock && sk_psock_test_state(psock, SK_PSOCK_TX_ENABLED)))
> Check for NULL
> 791 schedule_work(&psock->work);
> 792 write_space = psock->saved_write_space;
> ^^^^^^^^^^^^^^^^^^^^^^^^
> 793 rcu_read_unlock();
> 794 write_space(sk);
>
> Ensure psock dereference on line 792 only occurs if psock is not null.
>
> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
> Fixes: 604326b41a6f ("bpf, sockmap: convert to generic sk_msg interface")
> Signed-off-by: John Fastabend <john.fastabend@gmail.com>
lgtm.
John, do you feel strongly about it going to net tree asap?
Can it go to net-next ? The merge window is right around the corner.
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [net PATCH] bpf: skmsg, fix potential psock NULL pointer dereference
2019-11-21 20:15 ` Alexei Starovoitov
@ 2019-11-21 20:27 ` John Fastabend
2019-11-21 21:44 ` David Miller
0 siblings, 1 reply; 4+ messages in thread
From: John Fastabend @ 2019-11-21 20:27 UTC (permalink / raw
To: Alexei Starovoitov, John Fastabend
Cc: David S. Miller, Network Development, Dan Carpenter,
Daniel Borkmann
Alexei Starovoitov wrote:
> On Thu, Nov 21, 2019 at 8:28 AM John Fastabend <john.fastabend@gmail.com> wrote:
> >
> > Report from Dan Carpenter,
> >
> > net/core/skmsg.c:792 sk_psock_write_space()
> > error: we previously assumed 'psock' could be null (see line 790)
> >
> > net/core/skmsg.c
> > 789 psock = sk_psock(sk);
> > 790 if (likely(psock && sk_psock_test_state(psock, SK_PSOCK_TX_ENABLED)))
> > Check for NULL
> > 791 schedule_work(&psock->work);
> > 792 write_space = psock->saved_write_space;
> > ^^^^^^^^^^^^^^^^^^^^^^^^
> > 793 rcu_read_unlock();
> > 794 write_space(sk);
> >
> > Ensure psock dereference on line 792 only occurs if psock is not null.
> >
> > Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
> > Fixes: 604326b41a6f ("bpf, sockmap: convert to generic sk_msg interface")
> > Signed-off-by: John Fastabend <john.fastabend@gmail.com>
>
> lgtm.
> John, do you feel strongly about it going to net tree asap?
> Can it go to net-next ? The merge window is right around the corner.
Agree we can send it to net-next, its been in the kernel for multiple
versions anyways.
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [net PATCH] bpf: skmsg, fix potential psock NULL pointer dereference
2019-11-21 20:27 ` John Fastabend
@ 2019-11-21 21:44 ` David Miller
0 siblings, 0 replies; 4+ messages in thread
From: David Miller @ 2019-11-21 21:44 UTC (permalink / raw
To: john.fastabend; +Cc: alexei.starovoitov, netdev, dan.carpenter, daniel
From: John Fastabend <john.fastabend@gmail.com>
Date: Thu, 21 Nov 2019 12:27:23 -0800
> Alexei Starovoitov wrote:
>> On Thu, Nov 21, 2019 at 8:28 AM John Fastabend <john.fastabend@gmail.com> wrote:
>> >
>> > Report from Dan Carpenter,
>> >
>> > net/core/skmsg.c:792 sk_psock_write_space()
>> > error: we previously assumed 'psock' could be null (see line 790)
>> >
>> > net/core/skmsg.c
>> > 789 psock = sk_psock(sk);
>> > 790 if (likely(psock && sk_psock_test_state(psock, SK_PSOCK_TX_ENABLED)))
>> > Check for NULL
>> > 791 schedule_work(&psock->work);
>> > 792 write_space = psock->saved_write_space;
>> > ^^^^^^^^^^^^^^^^^^^^^^^^
>> > 793 rcu_read_unlock();
>> > 794 write_space(sk);
>> >
>> > Ensure psock dereference on line 792 only occurs if psock is not null.
>> >
>> > Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
>> > Fixes: 604326b41a6f ("bpf, sockmap: convert to generic sk_msg interface")
>> > Signed-off-by: John Fastabend <john.fastabend@gmail.com>
>>
>> lgtm.
>> John, do you feel strongly about it going to net tree asap?
>> Can it go to net-next ? The merge window is right around the corner.
>
> Agree we can send it to net-next, its been in the kernel for multiple
> versions anyways.
Applied to net-next, thanks.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2019-11-21 21:44 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-11-21 16:25 [net PATCH] bpf: skmsg, fix potential psock NULL pointer dereference John Fastabend
2019-11-21 20:15 ` Alexei Starovoitov
2019-11-21 20:27 ` John Fastabend
2019-11-21 21:44 ` David Miller
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.