* [PATCH net] net/bpfilter: initialize pos in __bpfilter_process_sockopt
@ 2020-07-30 16:09 Christoph Hellwig
2020-07-30 16:13 ` Christian Brauner
0 siblings, 1 reply; 6+ messages in thread
From: Christoph Hellwig @ 2020-07-30 16:09 UTC (permalink / raw)
To: davem, kuba, ast, daniel; +Cc: netdev, bpf, Christian Brauner, Rodrigo Madera
__bpfilter_process_sockopt never initialized the pos variable passed to
the pipe write. This has been mostly harmless in the past as pipes
ignore the offset, but the switch to kernel_write no verified the
position, which can lead to a failure depending on the exact stack
initialization patter. Initialize the variable to zero to make
rw_verify_area happy.
Fixes: 6955a76fbcd5 ("bpfilter: switch to kernel_write")
Reported-by: Christian Brauner <christian.brauner@ubuntu.com>
Reported-by: Rodrigo Madera <rodrigo.madera@gmail.com>
Signed-off-by: Christoph Hellwig <hch@lst.de>
Tested-by: Rodrigo Madera <rodrigo.madera@gmail.com>
---
net/bpfilter/bpfilter_kern.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/bpfilter/bpfilter_kern.c b/net/bpfilter/bpfilter_kern.c
index 1905e01c3aa9a7..4494ea6056cdb8 100644
--- a/net/bpfilter/bpfilter_kern.c
+++ b/net/bpfilter/bpfilter_kern.c
@@ -39,7 +39,7 @@ static int __bpfilter_process_sockopt(struct sock *sk, int optname,
{
struct mbox_request req;
struct mbox_reply reply;
- loff_t pos;
+ loff_t pos = 0;
ssize_t n;
int ret = -EFAULT;
--
2.27.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH net] net/bpfilter: initialize pos in __bpfilter_process_sockopt
2020-07-30 16:09 [PATCH net] net/bpfilter: initialize pos in __bpfilter_process_sockopt Christoph Hellwig
@ 2020-07-30 16:13 ` Christian Brauner
2020-07-31 0:07 ` Daniel Borkmann
0 siblings, 1 reply; 6+ messages in thread
From: Christian Brauner @ 2020-07-30 16:13 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: davem, kuba, ast, daniel, netdev, bpf, Rodrigo Madera
On Thu, Jul 30, 2020 at 06:09:00PM +0200, Christoph Hellwig wrote:
> __bpfilter_process_sockopt never initialized the pos variable passed to
> the pipe write. This has been mostly harmless in the past as pipes
> ignore the offset, but the switch to kernel_write no verified the
s/no/now/
> position, which can lead to a failure depending on the exact stack
> initialization patter. Initialize the variable to zero to make
s/patter/pattern/
> rw_verify_area happy.
>
> Fixes: 6955a76fbcd5 ("bpfilter: switch to kernel_write")
> Reported-by: Christian Brauner <christian.brauner@ubuntu.com>
> Reported-by: Rodrigo Madera <rodrigo.madera@gmail.com>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> Tested-by: Rodrigo Madera <rodrigo.madera@gmail.com>
> ---
Thanks for tracking this down, Christoph! This fixes the logging issue
for me.
Tested-by: Christian Brauner <christian.brauner@ubuntu.com>
Reviewed-by: Christian Brauner <christian.brauner@ubuntu.com>
> net/bpfilter/bpfilter_kern.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/bpfilter/bpfilter_kern.c b/net/bpfilter/bpfilter_kern.c
> index 1905e01c3aa9a7..4494ea6056cdb8 100644
> --- a/net/bpfilter/bpfilter_kern.c
> +++ b/net/bpfilter/bpfilter_kern.c
> @@ -39,7 +39,7 @@ static int __bpfilter_process_sockopt(struct sock *sk, int optname,
> {
> struct mbox_request req;
> struct mbox_reply reply;
> - loff_t pos;
> + loff_t pos = 0;
> ssize_t n;
> int ret = -EFAULT;
>
> --
> 2.27.0
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH net] net/bpfilter: initialize pos in __bpfilter_process_sockopt
2020-07-30 16:13 ` Christian Brauner
@ 2020-07-31 0:07 ` Daniel Borkmann
2020-08-01 19:48 ` Alexei Starovoitov
0 siblings, 1 reply; 6+ messages in thread
From: Daniel Borkmann @ 2020-07-31 0:07 UTC (permalink / raw)
To: Christian Brauner, Christoph Hellwig
Cc: davem, kuba, ast, netdev, bpf, Rodrigo Madera
On 7/30/20 6:13 PM, Christian Brauner wrote:
> On Thu, Jul 30, 2020 at 06:09:00PM +0200, Christoph Hellwig wrote:
>> __bpfilter_process_sockopt never initialized the pos variable passed to
>> the pipe write. This has been mostly harmless in the past as pipes
>> ignore the offset, but the switch to kernel_write no verified the
>
> s/no/now/
>
>> position, which can lead to a failure depending on the exact stack
>> initialization patter. Initialize the variable to zero to make
>
> s/patter/pattern/
>
>> rw_verify_area happy.
>>
>> Fixes: 6955a76fbcd5 ("bpfilter: switch to kernel_write")
>> Reported-by: Christian Brauner <christian.brauner@ubuntu.com>
>> Reported-by: Rodrigo Madera <rodrigo.madera@gmail.com>
>> Signed-off-by: Christoph Hellwig <hch@lst.de>
>> Tested-by: Rodrigo Madera <rodrigo.madera@gmail.com>
>> ---
>
> Thanks for tracking this down, Christoph! This fixes the logging issue
> for me.
> Tested-by: Christian Brauner <christian.brauner@ubuntu.com>
> Reviewed-by: Christian Brauner <christian.brauner@ubuntu.com>
Applied to bpf & fixed up the typos in the commit msg, thanks everyone!
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH net] net/bpfilter: initialize pos in __bpfilter_process_sockopt
2020-07-31 0:07 ` Daniel Borkmann
@ 2020-08-01 19:48 ` Alexei Starovoitov
2020-08-03 14:56 ` Daniel Borkmann
0 siblings, 1 reply; 6+ messages in thread
From: Alexei Starovoitov @ 2020-08-01 19:48 UTC (permalink / raw)
To: Daniel Borkmann
Cc: Christian Brauner, Christoph Hellwig, davem, kuba, ast, netdev,
bpf, Rodrigo Madera
On Fri, Jul 31, 2020 at 02:07:42AM +0200, Daniel Borkmann wrote:
> On 7/30/20 6:13 PM, Christian Brauner wrote:
> > On Thu, Jul 30, 2020 at 06:09:00PM +0200, Christoph Hellwig wrote:
> > > __bpfilter_process_sockopt never initialized the pos variable passed to
> > > the pipe write. This has been mostly harmless in the past as pipes
> > > ignore the offset, but the switch to kernel_write no verified the
> >
> > s/no/now/
> >
> > > position, which can lead to a failure depending on the exact stack
> > > initialization patter. Initialize the variable to zero to make
> >
> > s/patter/pattern/
> >
> > > rw_verify_area happy.
> > >
> > > Fixes: 6955a76fbcd5 ("bpfilter: switch to kernel_write")
> > > Reported-by: Christian Brauner <christian.brauner@ubuntu.com>
> > > Reported-by: Rodrigo Madera <rodrigo.madera@gmail.com>
> > > Signed-off-by: Christoph Hellwig <hch@lst.de>
> > > Tested-by: Rodrigo Madera <rodrigo.madera@gmail.com>
> > > ---
> >
> > Thanks for tracking this down, Christoph! This fixes the logging issue
> > for me.
> > Tested-by: Christian Brauner <christian.brauner@ubuntu.com>
> > Reviewed-by: Christian Brauner <christian.brauner@ubuntu.com>
>
> Applied to bpf & fixed up the typos in the commit msg, thanks everyone!
Daniel,
why is it necessary in bpf tree?
I fixed it already in bpf-next in commit a4fa458950b4 ("bpfilter: Initialize pos variable")
two weeks ago...
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH net] net/bpfilter: initialize pos in __bpfilter_process_sockopt
2020-08-01 19:48 ` Alexei Starovoitov
@ 2020-08-03 14:56 ` Daniel Borkmann
2020-08-03 15:36 ` Rodrigo Madera
0 siblings, 1 reply; 6+ messages in thread
From: Daniel Borkmann @ 2020-08-03 14:56 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Christian Brauner, Christoph Hellwig, davem, kuba, ast, netdev,
bpf, Rodrigo Madera
On 8/1/20 9:48 PM, Alexei Starovoitov wrote:
> On Fri, Jul 31, 2020 at 02:07:42AM +0200, Daniel Borkmann wrote:
>> On 7/30/20 6:13 PM, Christian Brauner wrote:
>>> On Thu, Jul 30, 2020 at 06:09:00PM +0200, Christoph Hellwig wrote:
>>>> __bpfilter_process_sockopt never initialized the pos variable passed to
>>>> the pipe write. This has been mostly harmless in the past as pipes
>>>> ignore the offset, but the switch to kernel_write no verified the
>>>
>>> s/no/now/
>>>
>>>> position, which can lead to a failure depending on the exact stack
>>>> initialization patter. Initialize the variable to zero to make
>>>
>>> s/patter/pattern/
>>>
>>>> rw_verify_area happy.
>>>>
>>>> Fixes: 6955a76fbcd5 ("bpfilter: switch to kernel_write")
>>>> Reported-by: Christian Brauner <christian.brauner@ubuntu.com>
>>>> Reported-by: Rodrigo Madera <rodrigo.madera@gmail.com>
>>>> Signed-off-by: Christoph Hellwig <hch@lst.de>
>>>> Tested-by: Rodrigo Madera <rodrigo.madera@gmail.com>
>>>> ---
>>>
>>> Thanks for tracking this down, Christoph! This fixes the logging issue
>>> for me.
>>> Tested-by: Christian Brauner <christian.brauner@ubuntu.com>
>>> Reviewed-by: Christian Brauner <christian.brauner@ubuntu.com>
>>
>> Applied to bpf & fixed up the typos in the commit msg, thanks everyone!
>
> Daniel,
> why is it necessary in bpf tree?
>
> I fixed it already in bpf-next in commit a4fa458950b4 ("bpfilter: Initialize pos variable")
> two weeks ago...
Several folks reported that with v5.8-rc kernels their console is spammed with
'bpfilter: write fail' messages [0]. Given this affected the 5.8 release and
the fix was a one-line change, it felt appropriate to route it there. Why was
a4fa458950b4 not pushed into bpf tree given it was affected there too? Either
way, we can undo the double pos assignment upon tree sync..
[0] https://lore.kernel.org/lkml/20200727104636.nuz3u4xb7ba7ue5a@wittgenstein/
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH net] net/bpfilter: initialize pos in __bpfilter_process_sockopt
2020-08-03 14:56 ` Daniel Borkmann
@ 2020-08-03 15:36 ` Rodrigo Madera
0 siblings, 0 replies; 6+ messages in thread
From: Rodrigo Madera @ 2020-08-03 15:36 UTC (permalink / raw)
To: Daniel Borkmann
Cc: Alexei Starovoitov, Christian Brauner, Christoph Hellwig, davem,
kuba, ast, netdev, bpf
On Mon, Aug 03, 2020 at 04:56:35PM +0200, Daniel Borkmann wrote:
> On 8/1/20 9:48 PM, Alexei Starovoitov wrote:
>
> Several folks reported that with v5.8-rc kernels their console is spammed with
> 'bpfilter: write fail' messages [0]. Given this affected the 5.8 release and
> the fix was a one-line change, it felt appropriate to route it there. Why was
> a4fa458950b4 not pushed into bpf tree given it was affected there too? Either
> way, we can undo the double pos assignment upon tree sync..
Just as a side note, please note it was more than spamming on the console.
It prevented the subsystem from working at all.
Best regards,
Madera
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-08-03 15:36 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-30 16:09 [PATCH net] net/bpfilter: initialize pos in __bpfilter_process_sockopt Christoph Hellwig
2020-07-30 16:13 ` Christian Brauner
2020-07-31 0:07 ` Daniel Borkmann
2020-08-01 19:48 ` Alexei Starovoitov
2020-08-03 14:56 ` Daniel Borkmann
2020-08-03 15:36 ` Rodrigo Madera
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).