Re: WARNING in __proc_create

[Eric Dumazet]

On 03/09/2018 03:32 PM, Cong Wang wrote:

On Fri, Mar 9, 2018 at 3:21 PM, Eric Dumazet  wrote:



On 03/09/2018 03:05 PM, Cong Wang wrote:



BTW, the warning itself is all about empty names, so perhaps
it's better to fix them separately.



Huh ? You want more syzbot reports ? I do not.


I always prefer one patch to fix one problem, and, as I already said,
checking "." and ".." is probably not as trivial as checking empty.

This why I believe 2 (or more) patches here are better than 1 single
patch.

BTW, I don't have any patch, it is so trivial that I don't want to spend
my time on it.


The trivial patch has been sent, and Florian gave a (very good) feedback.

Then Pablo offered to write a complete patch.

They both had more pressing issues, and quite frankly I also had more 
pressing issues.


I doubt that sending again the simple fix will be accepted by netfilter 
maintainers.

Re: WARNING in __proc_create

[Cong Wang]

On Fri, Mar 9, 2018 at 3:21 PM, Eric Dumazet  wrote:
>
>
> On 03/09/2018 03:05 PM, Cong Wang wrote:
>>
>>
>> BTW, the warning itself is all about empty names, so perhaps
>> it's better to fix them separately.
>
>
> Huh ? You want more syzbot reports ? I do not.

I always prefer one patch to fix one problem, and, as I already said,
checking "." and ".." is probably not as trivial as checking empty.

This why I believe 2 (or more) patches here are better than 1 single
patch.

BTW, I don't have any patch, it is so trivial that I don't want to spend
my time on it.

>
> I unblocked this report today [1], you can be sure that as soon
> as syzbot gets the correct tag (Reported-by: ...), it will find the other
> problems, which are currently on hold to avoid spam.
>
> [1] It has been sitting for a while here at Google, since January 28th.
>At some point you have to ping Pablo/Florian again ;)

Sure, it can always find more problems... why just complain about
this one alone? ;)

Re: WARNING in __proc_create

[Eric Dumazet]

On 03/09/2018 03:05 PM, Cong Wang wrote:


BTW, the warning itself is all about empty names, so perhaps
it's better to fix them separately.


Huh ? You want more syzbot reports ? I do not.

I unblocked this report today [1], you can be sure that as soon
as syzbot gets the correct tag (Reported-by: ...), it will find the 
other problems, which are currently on hold to avoid spam.


[1] It has been sitting for a while here at Google, since January 28th.
   At some point you have to ping Pablo/Florian again ;)

Re: WARNING in __proc_create

[Florian Westphal]

Cong Wang  wrote:
> On Fri, Mar 9, 2018 at 2:58 PM, Eric Dumazet  wrote:
> >
> >
> > On 03/09/2018 02:56 PM, Eric Dumazet wrote:
> >
> >>
> >> I sent a patch a while back, but Pablo/Florian wanted more than that
> >> simple fix.
> >>
> >> We also need to filter special characters like '/'
> 
> proc_create_data() itself accepts '/', so it must be xt_hashlimit doesn't
> want it.

--hashimit-name / also triggers WARN for me.
. or .. "work", (no crash), but cause appearance of 2nd ./.. in
/proc/net/ipt_hashlimit , so I think its better to disallow that too.

Re: WARNING in __proc_create

[Cong Wang]

On Fri, Mar 9, 2018 at 2:58 PM, Eric Dumazet  wrote:
>
>
> On 03/09/2018 02:56 PM, Eric Dumazet wrote:
>
>>
>> I sent a patch a while back, but Pablo/Florian wanted more than that
>> simple fix.
>>
>> We also need to filter special characters like '/'

proc_create_data() itself accepts '/', so it must be xt_hashlimit doesn't
want it.

>>
>> Or maybe I am mixing with something else.
>
>
> Yes, Florian mentioned that we also had to reject "."  and ".."
>

It could be, but looks like not as trivial as just strstr(".")?

BTW, the warning itself is all about empty names, so perhaps
it's better to fix them separately.

Re: WARNING in __proc_create

[Florian Westphal]

Eric Dumazet  wrote:
> >>fs/proc/generic.c:354
> >
> >We need to reject empty names.
> >
> 
> I sent a patch a while back, but Pablo/Florian wanted more than that simple
> fix.
> 
> We also need to filter special characters like '/'
> 
> Or maybe I am mixing with something else.

Argh, sorry, this fell off the truck it seems :(

I'll work on this tomorrow unless someone else does it today ;)

Re: WARNING in __proc_create

[Eric Dumazet]

On 03/09/2018 02:56 PM, Eric Dumazet wrote:



I sent a patch a while back, but Pablo/Florian wanted more than that 
simple fix.


We also need to filter special characters like '/'

Or maybe I am mixing with something else.


Yes, Florian mentioned that we also had to reject "."  and ".."

Re: WARNING in __proc_create

[Eric Dumazet]

On 03/09/2018 02:48 PM, Cong Wang wrote:

On Fri, Mar 9, 2018 at 1:59 PM, syzbot
 wrote:

Hello,

syzbot hit the following crash on net-next commit
617aebe6a97efa539cc4b8a52adccd89596e6be0 (Sun Feb 4 00:25:42 2018 +)
Merge tag 'usercopy-v4.16-rc1' of
git://git.kernel.org/pub/scm/linux/kernel/git/kees/linux

So far this crash happened 12 times on net-next, upstream.
C reproducer is attached.
syzkaller reproducer is attached.
Raw console output is attached.
compiler: gcc (GCC) 7.1.1 20170620
.config is attached.

IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: syzbot+0502b00edac2a0680...@syzkaller.appspotmail.com
It will help syzbot understand when the bug is fixed. See footer for
details.
If you forward the report, please keep this part and the footer.

audit: type=1400 audit(1518223777.419:7): avc:  denied  { map } for
pid=4159 comm="syzkaller598581" path="/root/syzkaller598581546" dev="sda1"
ino=16481 scontext=unconfined_u:system_r:insmod_t:s0-s0:c0.c1023
tcontext=unconfined_u:object_r:user_home_t:s0 tclass=file permissive=1
[ cut here ]
name len 0
WARNING: CPU: 0 PID: 4159 at fs/proc/generic.c:354 __proc_create+0x696/0x880
fs/proc/generic.c:354


We need to reject empty names.



I sent a patch a while back, but Pablo/Florian wanted more than that 
simple fix.


We also need to filter special characters like '/'

Or maybe I am mixing with something else.

Re: WARNING in __proc_create

[Cong Wang]

On Fri, Mar 9, 2018 at 1:59 PM, syzbot
 wrote:
> Hello,
>
> syzbot hit the following crash on net-next commit
> 617aebe6a97efa539cc4b8a52adccd89596e6be0 (Sun Feb 4 00:25:42 2018 +)
> Merge tag 'usercopy-v4.16-rc1' of
> git://git.kernel.org/pub/scm/linux/kernel/git/kees/linux
>
> So far this crash happened 12 times on net-next, upstream.
> C reproducer is attached.
> syzkaller reproducer is attached.
> Raw console output is attached.
> compiler: gcc (GCC) 7.1.1 20170620
> .config is attached.
>
> IMPORTANT: if you fix the bug, please add the following tag to the commit:
> Reported-by: syzbot+0502b00edac2a0680...@syzkaller.appspotmail.com
> It will help syzbot understand when the bug is fixed. See footer for
> details.
> If you forward the report, please keep this part and the footer.
>
> audit: type=1400 audit(1518223777.419:7): avc:  denied  { map } for
> pid=4159 comm="syzkaller598581" path="/root/syzkaller598581546" dev="sda1"
> ino=16481 scontext=unconfined_u:system_r:insmod_t:s0-s0:c0.c1023
> tcontext=unconfined_u:object_r:user_home_t:s0 tclass=file permissive=1
> [ cut here ]
> name len 0
> WARNING: CPU: 0 PID: 4159 at fs/proc/generic.c:354 __proc_create+0x696/0x880
> fs/proc/generic.c:354

We need to reject empty names.