linux-snps-arc.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: Vineet Gupta <vgupta@kernel.org>
To: Rick Edgecombe <rick.p.edgecombe@intel.com>
Cc: Liam.Howlett@oracle.com, akpm@linux-foundation.org, bp@alien8.de,
	broonie@kernel.org, dave.hansen@linux.intel.com,
	debug@rivosinc.com, hpa@zytor.com, keescook@chromium.org,
	kirill.shutemov@linux.intel.com, luto@kernel.org,
	mingo@redhat.com, peterz@infradead.org,
	sparclinux@vger.kernel.org, tglx@linutronix.de, x86@kernel.org,
	Vineet Gupta <vgupta@kernel.org>,
	linux-snps-arc@lists.infradead.org
Subject: Re: [RFC v2.1 01/12] ARC: Use initializer for struct vm_unmapped_area_info
Date: Fri, 1 Mar 2024 20:42:01 -0800	[thread overview]
Message-ID: <5132da26-7c2a-4269-aa71-17315593dbde@kernel.org> (raw)
In-Reply-To: <20240302001714.674091-1-rick.p.edgecombe@intel.com>



On 3/1/24 16:17, Rick Edgecombe wrote:
> Future changes will need to add a new member to struct
> vm_unmapped_area_info. This would cause trouble for any call site that
> doesn't initialize the struct. Currently every caller sets each field
> manually, so if new fields are added they will be unitialized and the core
> code parsing the struct will see garbage in the new field.
>
> It could be possible to initialize the new field manually to 0 at each
> call site. This and a couple other options were discussed, and the
> consensus (see links) was that in general the best way to accomplish this
> would be via static initialization with designated field initiators.
> Having some struct vm_unmapped_area_info instances not zero initialized
> will put those sites at risk of feeding garbage into vm_unmapped_area() if
> the convention is to zero initialize the struct and any new field addition
> misses a call site that initializes each field manually.
>
> It could be possible to leave the code mostly untouched, and just change
> the line:
> struct vm_unmapped_area_info info
> to:
> struct vm_unmapped_area_info info = {};
>
> However, that would leave cleanup for the fields that are manually set
> to zero, as it would no longer be required.
>
> So to be reduce the chance of bugs via uninitialized fields, instead
> simply continue the process to initialize the struct this way tree wide.
> This will zero any unspecified members. Move the field initializers to the
> struct declaration when they are known at that time. Leave the fields out
> that were manually initialized to zero, as this would be redundant for
> designated initializers.
>
> Signed-off-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
> Cc: Vineet Gupta <vgupta@kernel.org>
> Cc: linux-snps-arc@lists.infradead.org
> Link: https://lore.kernel.org/lkml/202402280912.33AEE7A9CF@keescook/#t
> Link: https://lore.kernel.org/lkml/j7bfvig3gew3qruouxrh7z7ehjjafrgkbcmg6tcghhfh3rhmzi@wzlcoecgy5rs/

LGTM.

Acked-by: Vineet Gupta <vgupta@kernel.org>

Thx,
-Vineet

_______________________________________________
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc

      reply	other threads:[~2024-03-02  4:42 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20240226190951.3240433-1-rick.p.edgecombe@intel.com>
2024-02-26 19:09 ` [PATCH v2 5/9] mm: Initialize struct vm_unmapped_area_info Rick Edgecombe
2024-02-27  7:02   ` Christophe Leroy
2024-02-27 15:00     ` Edgecombe, Rick P
2024-02-27 18:07     ` Kees Cook
2024-02-27 18:16       ` Christophe Leroy
2024-02-27 20:25         ` Edgecombe, Rick P
2024-02-28 13:22           ` Christophe Leroy
2024-02-28 17:01             ` Edgecombe, Rick P
2024-02-28 23:10               ` Christophe Leroy
2024-02-28 17:21             ` Kees Cook
2024-03-02  0:47               ` Edgecombe, Rick P
2024-03-02  1:51                 ` Kees Cook
2024-03-04 18:00                   ` Christophe Leroy
2024-03-04 18:03                     ` Edgecombe, Rick P
2024-02-28 11:51   ` Kirill A. Shutemov
2024-03-02  0:17   ` [RFC v2.1 01/12] ARC: Use initializer for " Rick Edgecombe
2024-03-02  4:42     ` Vineet Gupta [this message]

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=5132da26-7c2a-4269-aa71-17315593dbde@kernel.org \
    --to=vgupta@kernel.org \
    --cc=Liam.Howlett@oracle.com \
    --cc=akpm@linux-foundation.org \
    --cc=bp@alien8.de \
    --cc=broonie@kernel.org \
    --cc=dave.hansen@linux.intel.com \
    --cc=debug@rivosinc.com \
    --cc=hpa@zytor.com \
    --cc=keescook@chromium.org \
    --cc=kirill.shutemov@linux.intel.com \
    --cc=linux-snps-arc@lists.infradead.org \
    --cc=luto@kernel.org \
    --cc=mingo@redhat.com \
    --cc=peterz@infradead.org \
    --cc=rick.p.edgecombe@intel.com \
    --cc=sparclinux@vger.kernel.org \
    --cc=tglx@linutronix.de \
    --cc=x86@kernel.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).