BPF List
 help / color / mirror / Atom feed
* [bug report] bpf, sockmap: convert to generic sk_msg interface
@ 2019-10-14 12:13 Dan Carpenter
  2019-10-14 16:46 ` Martin Lau
  0 siblings, 1 reply; 3+ messages in thread
From: Dan Carpenter @ 2019-10-14 12:13 UTC (permalink / raw)
  To: daniel; +Cc: bpf

Hello Daniel Borkmann,

This is a semi-automatic email about new static checker warnings.

The patch 604326b41a6f: "bpf, sockmap: convert to generic sk_msg
interface" from Oct 13, 2018, leads to the following Smatch complaint:

    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);
                ^^^^^^^^^^^^^^
The warning is on the wrong line.  The dereference is really here.

regards,
dan carpenter

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [bug report] bpf, sockmap: convert to generic sk_msg interface
  2019-10-14 12:13 [bug report] bpf, sockmap: convert to generic sk_msg interface Dan Carpenter
@ 2019-10-14 16:46 ` Martin Lau
  2019-10-14 16:53   ` John Fastabend
  0 siblings, 1 reply; 3+ messages in thread
From: Martin Lau @ 2019-10-14 16:46 UTC (permalink / raw)
  To: John Fastabend; +Cc: daniel@iogearbox.net, bpf@vger.kernel.org, Dan Carpenter

On Mon, Oct 14, 2019 at 03:13:30PM +0300, Dan Carpenter wrote:
> Hello Daniel Borkmann,
> 
> This is a semi-automatic email about new static checker warnings.
> 
> The patch 604326b41a6f: "bpf, sockmap: convert to generic sk_msg
> interface" from Oct 13, 2018, leads to the following Smatch complaint:
> 
>     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);
>                 ^^^^^^^^^^^^^^
> The warning is on the wrong line.  The dereference is really here.
> 
> regards,
> dan carpenter
John, can you also take a look?

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [bug report] bpf, sockmap: convert to generic sk_msg interface
  2019-10-14 16:46 ` Martin Lau
@ 2019-10-14 16:53   ` John Fastabend
  0 siblings, 0 replies; 3+ messages in thread
From: John Fastabend @ 2019-10-14 16:53 UTC (permalink / raw)
  To: Martin Lau, John Fastabend
  Cc: daniel@iogearbox.net, bpf@vger.kernel.org, Dan Carpenter

Martin Lau wrote:
> On Mon, Oct 14, 2019 at 03:13:30PM +0300, Dan Carpenter wrote:
> > Hello Daniel Borkmann,
> > 
> > This is a semi-automatic email about new static checker warnings.
> > 
> > The patch 604326b41a6f: "bpf, sockmap: convert to generic sk_msg
> > interface" from Oct 13, 2018, leads to the following Smatch complaint:
> > 
> >     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);
> >                 ^^^^^^^^^^^^^^
> > The warning is on the wrong line.  The dereference is really here.
> > 
> > regards,
> > dan carpenter
> John, can you also take a look?

Thanks Dan, seems there is a chance we could have a null psock here
so we need to wrap both dereferences in the 'if (psock)'. I'll send
a patch.

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2019-10-14 16:53 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-10-14 12:13 [bug report] bpf, sockmap: convert to generic sk_msg interface Dan Carpenter
2019-10-14 16:46 ` Martin Lau
2019-10-14 16:53   ` John Fastabend

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox