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
| ||
|
Message-ID: <8533dc25-1cfe-bc0c-98e2-3db1b1c5c72d@iogearbox.net> Date: Tue, 19 Dec 2023 17:36:38 +0100 From: Daniel Borkmann <daniel@...earbox.net> To: Christian Brauner <brauner@...nel.org>, Linus Torvalds <torvalds@...uxfoundation.org>, Alexei Starovoitov <alexei.starovoitov@...il.com>, andrii@...nel.org Cc: kuba@...nel.org, davem@...emloft.net, edumazet@...gle.com, pabeni@...hat.com, peterz@...radead.org, netdev@...r.kernel.org, bpf@...r.kernel.org, kernel-team@...com, linux-fsdevel@...r.kernel.org Subject: Re: pull-request: bpf-next 2023-12-18 On 12/19/23 11:23 AM, Christian Brauner wrote: > On Mon, Dec 18, 2023 at 05:11:23PM -0800, Linus Torvalds wrote: >> On Mon, 18 Dec 2023 at 16:05, Alexei Starovoitov >> <alexei.starovoitov@...il.com> wrote: >>> >>> 2) Introduce BPF token object, from Andrii Nakryiko. >> >> I assume this is why I and some other unusual recipients are cc'd, >> because the networking people feel like they can't judge this and >> shouldn't merge non-networking code like this. >> >> Honestly, I was told - and expected - that this part would come in a >> branch of its own, so that it would be sanely reviewable. >> >> Now it's mixed in with everything else. >> >> This is *literally* why we have branches in git, so that people can >> make more independent changes and judgements, and so that we don't >> have to be in a situation where "look, here's ten different things, >> pull it all or nothing". >> >> Many of the changes *look* like they are in branches, but they've been >> the "fake branches" that are just done as "patch series in a branch, >> with the cover letter as the merge message". >> >> Which is great for maintaining that cover letter information and a >> certain amount of historical clarity, but not helpful AT ALL for the >> "independent changes" thing when it is all mixed up in history, where >> independent things are mostly serialized and not actually independent >> in history at all. >> >> So now it appears to be one big mess, and exactly that "all or >> nothing" thing that isn't great, since the whole point was that the >> networking people weren't comfortable with the reviewing filesystem >> side. >> >> And honestly, the bpf side *still* seems to be absolutely conbfused >> and complkete crap when it comes to file descriptors. >> >> I took a quick look, and I *still* see new code being introduced there >> that thinks that file descriptor zero is special, and we tols you a >> *year* ago that that wasn't true, and that you need to fix this. >> >> I literally see complete garbage like tghis: >> >> .. >> __u32 btf_token_fd; >> ... >> if (attr->btf_token_fd) { >> token = bpf_token_get_from_fd(attr->btf_token_fd); >> >> and this is all *new* code that makes that same bogus sh*t-for-brains >> mistake that was wrong the first time. >> >> So now I'm saying NAK. Enough is enough. No more of this crazy "I >> don't understand even the _basics_ of file descriptors, and yet I'm >> introducing new random interfaces". >> >> I know you thought fd zero was something invalid. You were told >> otherwise. Apparently you just ignored being wrong, and have decided >> to double down on being wrong. >> >> We don't take this kind of flat-Earther crap. >> >> File descriptors don't start at 1. Deal with reality. Stop making the >> same mistake over and over. If you ant to have a "no file descriptor" >> flag, you use a signed type, and a signed value for that, because file >> descriptor zero is perfectly valid, and I don't want to hear any more >> uninformed denialism. >> >> Stop polluting the kernel with incorrect assumptions. >> >> So yes, I will keep NAK'ing this until this kind of fundamental >> mistake is fixed. This is not rocket science, and this is not >> something that wasn't discussed before. Your ignorance has now turned >> from "I didn't know" to "I didn 't care", and at that point I really >> don't want to see new code any more. > > Alexei, Andrii, this is a massive breach of trust and flatout > disrespectful. I barely reword mails and believe me I've reworded this > mail many times. I'm furious. > > Over the last couple of months since LSFMM in May 2023 until almost last > week I've given you extensive design and review for this whole approach > to get this into even remotely sane shape from a VFS perspective. > > The VFS maintainers including Linus have explicitly NAKed this "zero is > not a valid fd nonsense" and told you to stop doing that. We told you > that such fundamental VFS semantics are not yours to decide. > > And yet you put a patch into a series that did exactly that and then had > the unbelievable audacity to repeatedly ask me to put my ACK under this > - both in person and on list. > > I'm glad I only gave my ACK to the two patches that I extensivly > reviewed and never to the whole series. Sincere apologies for this whole mess. All token series related patches have been reverted in bpf-next now, and I'm prepping a PR for net-next so that this is also fully removed from there. Thanks, Daniel
Powered by blists - more mailing lists