Re: [PATCH] pipe_fs_i.h: add pipe_buf_init()

[Max Kellermann <max.kellermann@ionos.com>]

On Tue, Sep 19, 2023 at 4:16 PM Christian Brauner <brauner@kernel.org> wrote:
> You're changing how the code currently works which is written in a way
> that ensures all fields are initialized to zero. The fact that currently
> nothing looks at private is irrelevant.

Two callers were previously using a designated intiializer which
implicitly zero-initializes unmentioned fields; but that was not
intentional, but accidental. It is true that these two call sites are
changed, now omitting the implicit (and unintended) initializer. If
you consider this a problem, I'll re-add it to those two callers. But
adding the "private" initializer to the new function would also change
how the code currently works - fot the other callers.

It's only relevant if the "private" field is part of some API
contract. If that API contract is undocumented, we should add
documentation - what is it? I'll write documentation.

> Following your argument below this might very easily be the cause for
> another CVE when something starts looking at this.

When something starts looking at this, the API contract changes, and
this one function needs to be adjusted.

Without this function, there is not one function, but an arbitrary
number of redundant copies of this initializing code which all need to
be fixed. As I said, only two copies of those currently do initialize
"private", the others do not. Therefore, my patch is strictly an
improvement, and is safer against those alleged future CVEs, which is
the very goal of my patch.

> Wouldn't it make more sense to have the pipe_buf_init() initialize the
> whole thing and for the place where it leaves buf->private untouched you
> can just do:

Would it? I don't think so, but if you insist, I'll add it.
I prefer to leave "private" uninitialized unless there is a reason to
initialize it, for the reason I stated in my previous reply.