netdev - Re: [PATCH bpf 0/2] Two bpf fixes

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <fde8ceaa-3422-eaff-d02c-6c3e0c9137aa@iogearbox.net>
Date:   Fri, 15 Jun 2018 23:16:25 +0200
From:   Daniel Borkmann <daniel@...earbox.net>
To:     Alexei Starovoitov <alexei.starovoitov@...il.com>
Cc:     ast@...nel.org, netdev@...r.kernel.org
Subject: Re: [PATCH bpf 0/2] Two bpf fixes

On 06/15/2018 09:31 PM, Alexei Starovoitov wrote:
> On Fri, Jun 15, 2018 at 02:30:46AM +0200, Daniel Borkmann wrote:
>> First one is a panic I ran into while testing the second
>> one where we got several syzkaller reports. Series here
>> fixes both.
>>
>> Thanks!
> 
> Applied, thanks.
> 
> The second patch looks dubious to me though.
> Nothing in the kernel tree checks the return value of set_memory_ro()
> and my understanding that it can fail only when part of huge page
> is being marked and pages have to be split. In bpf case I don't think
> it's ever the case, so the patch is silencing purely theoretical
> syzbot splat that can happen with artificial error injection.
> I bet we're still going to see this splat in set_memory_rw.
> imo the better fix would have been to drop WARN_ON from both.

I think it should be pretty unlikely to trigger them in real world,
we have them in place for ~4yrs now as fixed-builtin and I haven't
heard any issues with it so far aside from the syzkaller splats which
triggered it with a total of 54 times via fault injection, fwiw.
Dropping second warn doesn't make sense actually since if we ever
run into it there's no option to recover, so we would want to know
where it breaks first.

Thanks,
Daniel