All of lore.kernel.org
 help / color / mirror / Atom feed
From: Christian Brauner <brauner@kernel.org>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Kees Cook <keescook@chromium.org>, linux-kernel@vger.kernel.org
Subject: Re: [GIT PULL] pid: use flex array
Date: Fri, 30 Jun 2023 08:51:17 +0200	[thread overview]
Message-ID: <20230630-testphasen-orangen-0e54486a267d@brauner> (raw)
In-Reply-To: <CAHk-=wiBXJOzkez2Rd=cQ5ckttJq6OdYtArFmCtVQHyeuQBGrw@mail.gmail.com>

On Thu, Jun 29, 2023 at 04:52:43PM -0700, Linus Torvalds wrote:
> On Wed, 28 Jun 2023 at 03:37, Christian Brauner <brauner@kernel.org> wrote:
> >
> > This contains Kees' work to make struct upid in struct pid a proper
> > flexible array and thus gets rid of a bunch of syzbot UBSAN warnings.
> 
> Hmm. Of this, about half were replacing "array + index" with "&array[index]".
> 
> Honestly, it makes no difference, but the reverse is also true: the
> "array + index" is *very* traditional, and if people have problems
> with that simple syntax I really don't know what to say. It's kind of
> core C. It's *literally* how arrays work, and what the '[]' operator
> means.

I have no preference for either syntax. Both work. But this is probably
more an objection to this being mixed in with the flex array change in
the first place.

> 
> And of the remaining half, half again is using a truly disgusting
> 
>     struct_size((struct pid *)0, numbers, X)

I did react to that in the original review here:
https://lore.kernel.org/all/20230518-zuneigen-brombeeren-0a57cd32b1a7@brauner
but then I grepped for it and saw it done in a few other places already
which is why I didn't ask for it to be changed. See commits
48658213202c ("scsi: megaraid_sas: Use struct_size() in code related to struct MR_PD_CFG_SEQ_NUM_SYNC") 
5b12a568cc6f ("scsi: hptiop: Use struct_size() helper in code related to struct hpt_iop_request_scsi_command"
as examples.

> 
> thing. That is *GARBAGE*. It's garbage for too many reasons for me to
> actually pull this sh*t, but let me just name them:
> 
>  - 0 isn't a pointer. Stop doing that.
> 
>  - dammit, we have 'struct_size_t' that does the above disgusting cast
> without getting that simple thing wrong.
> 
> In other words, this pull request contained half pointless and
> unrelated churn, and 25% actual garbage.
> 
> In other words, I'm not pulling this to just get the remaining 25%.

Sure. @Kees, I'd appreciate it if you could change the patch according
to the comments here.

  reply	other threads:[~2023-06-30  6:51 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-06-28 10:37 [GIT PULL] pid: use flex array Christian Brauner
2023-06-29 23:52 ` Linus Torvalds
2023-06-30  6:51   ` Christian Brauner [this message]
2023-06-30  7:12     ` Linus Torvalds
2023-06-30  8:04       ` Christian Brauner
2023-06-30 11:21         ` David Laight
2023-06-30 16:01           ` Linus Torvalds
2023-06-30 16:59         ` Kees Cook
2023-07-01  6:27           ` Christian Brauner
2023-07-01  6:44           ` [PATCH] pid: use struct_size_t() helper Christian Brauner
2023-07-01 15:28             ` Linus Torvalds

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230630-testphasen-orangen-0e54486a267d@brauner \
    --to=brauner@kernel.org \
    --cc=keescook@chromium.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=torvalds@linux-foundation.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.