All of lore.kernel.org
 help / color / mirror / Atom feed
From: David Teigland <teigland@redhat.com>
To: cluster-devel.redhat.com
Subject: [Cluster-devel] [PATCH v2][next] dlm: Replace one-element array with flexible-array member
Date: Mon, 10 Oct 2022 16:00:39 -0500	[thread overview]
Message-ID: <20221010210039.GA30273@redhat.com> (raw)
In-Reply-To: <202210082044.51106145BD@keescook>

On Sat, Oct 08, 2022 at 09:03:28PM -0700, Kees Cook wrote:
> On Sun, Oct 09, 2022 at 03:05:17PM +1300, Paulo Miguel Almeida wrote:
> > On Sat, Oct 08, 2022 at 05:18:35PM -0700, Kees Cook wrote:
> > > This is allocating 1 more byte than before, since the struct size didn't change. But this has always allocated too much space, due to the struct padding. For a "no binary changes" patch, the above "+ 1" needs to be left off.
> > 
> > That's true. I agree that leaving "+ 1" would work and produce a
> > no-binary-changes patch due to the existing padding that the structure
> > has. OTOH, I thought that relying on that space could bite us in the
> > future if anyone tweaks the struct again...so my reaction was to ensure 
> > that the NUL-terminator space was always guaranteed to be there.
> > Hence, the change on c693 (objdump above).
> > 
> > What do you think? Should we keep or leave the above
> > "+ 1" after the rationale above?
> 
> I think it depends on what's expected from this allocation. Christine or
> David, can you speak to this?

Hi, thanks for picking through that.  Most likely the intention was to
allow up to 64 (DLM_LOCKSPACE_LEN) character names, and then use the
ls_name[1] for the terminating byte.  I'd be happy to take the patch
replacing the one-element name.  Or, if you'd like to drop it, then we'll
eliminate it along with a cleanup of name/namelen more broadly.

Dave

WARNING: multiple messages have this Message-ID (diff)
From: David Teigland <teigland@redhat.com>
To: Kees Cook <keescook@chromium.org>
Cc: Paulo Miguel Almeida <paulo.miguel.almeida.rodenas@gmail.com>,
	Christine Caulfield <ccaulfie@redhat.com>,
	cluster-devel@redhat.com, linux-kernel@vger.kernel.org,
	linux-hardening@vger.kernel.org
Subject: Re: [PATCH v2][next] dlm: Replace one-element array with flexible-array member
Date: Mon, 10 Oct 2022 16:00:39 -0500	[thread overview]
Message-ID: <20221010210039.GA30273@redhat.com> (raw)
In-Reply-To: <202210082044.51106145BD@keescook>

On Sat, Oct 08, 2022 at 09:03:28PM -0700, Kees Cook wrote:
> On Sun, Oct 09, 2022 at 03:05:17PM +1300, Paulo Miguel Almeida wrote:
> > On Sat, Oct 08, 2022 at 05:18:35PM -0700, Kees Cook wrote:
> > > This is allocating 1 more byte than before, since the struct size didn't change. But this has always allocated too much space, due to the struct padding. For a "no binary changes" patch, the above "+ 1" needs to be left off.
> > 
> > That's true. I agree that leaving "+ 1" would work and produce a
> > no-binary-changes patch due to the existing padding that the structure
> > has. OTOH, I thought that relying on that space could bite us in the
> > future if anyone tweaks the struct again...so my reaction was to ensure 
> > that the NUL-terminator space was always guaranteed to be there.
> > Hence, the change on c693 (objdump above).
> > 
> > What do you think? Should we keep or leave the above
> > "+ 1" after the rationale above?
> 
> I think it depends on what's expected from this allocation. Christine or
> David, can you speak to this?

Hi, thanks for picking through that.  Most likely the intention was to
allow up to 64 (DLM_LOCKSPACE_LEN) character names, and then use the
ls_name[1] for the terminating byte.  I'd be happy to take the patch
replacing the one-element name.  Or, if you'd like to drop it, then we'll
eliminate it along with a cleanup of name/namelen more broadly.

Dave


  reply	other threads:[~2022-10-10 21:00 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-10-08 23:17 [Cluster-devel] [PATCH v2][next] dlm: Replace one-element array with flexible-array member Paulo Miguel Almeida
2022-10-08 23:17 ` Paulo Miguel Almeida
2022-10-09  0:18 ` [Cluster-devel] " Kees Cook
2022-10-09  0:18   ` Kees Cook
2022-10-09  2:05   ` [Cluster-devel] " Paulo Miguel Almeida
2022-10-09  2:05     ` Paulo Miguel Almeida
2022-10-09  4:03     ` [Cluster-devel] " Kees Cook
2022-10-09  4:03       ` Kees Cook
2022-10-10 21:00       ` David Teigland [this message]
2022-10-10 21:00         ` David Teigland
2022-10-10 22:35         ` [Cluster-devel] " Kees Cook
2022-10-10 22:35           ` Kees Cook
2022-10-11 15:20           ` [Cluster-devel] " David Teigland
2022-10-11 15:20             ` David Teigland
2022-10-11 18:44             ` [Cluster-devel] " Paulo Miguel Almeida
2022-10-11 18:44               ` Paulo Miguel Almeida
2022-10-11 20:04               ` [Cluster-devel] [PATCH v4] [next] dlm: replace one-element array with fixed size array Paulo Miguel Almeida
2022-10-11 20:04                 ` Paulo Miguel Almeida
2022-10-11 20:06                 ` [Cluster-devel] " Kees Cook
2022-10-11 20:06                   ` Kees Cook
2022-10-11 20:11                   ` [Cluster-devel] " Paulo Miguel Almeida
2022-10-11 20:11                     ` Paulo Miguel Almeida
2022-10-11 20:23                   ` [Cluster-devel] [PATCH v5] " Paulo Miguel Almeida
2022-10-11 20:23                     ` Paulo Miguel Almeida
2022-10-11 20:26                     ` [Cluster-devel] " Gustavo A. R. Silva
2022-10-11 20:26                       ` Gustavo A. R. Silva
2022-10-11 22:18                     ` [Cluster-devel] " Kees Cook
2022-10-11 22:18                       ` Kees Cook
2022-11-04  5:00                     ` [Cluster-devel] " Paulo Miguel Almeida
2022-11-04  5:00                       ` Paulo Miguel Almeida
2022-11-04 17:50                       ` [Cluster-devel] " Alexander Aring
2022-11-04 17:50                         ` Alexander Aring

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=20221010210039.GA30273@redhat.com \
    --to=teigland@redhat.com \
    /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.