* [PATCH] docs: deprecated.rst: Add note to the use of struct_size() helper @ 2020-06-04 17:21 Gustavo A. R. Silva 2020-06-04 17:49 ` Kees Cook 0 siblings, 1 reply; 6+ messages in thread From: Gustavo A. R. Silva @ 2020-06-04 17:21 UTC (permalink / raw) To: Jonathan Corbet, Kees Cook; +Cc: linux-doc, linux-kernel, Gustavo A. R. Silva Add a note to educate people about the proper use of struct_size() when the trailing array in the enclosing structure is a one-element array. Zero-length and one-element arrays will soon be removed from the kernel, but in the meantime, it's worth letting people know how to correctly use struct_size() together with such constructs. Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org> --- Kees, This is not substitute for the patch I'll write about flexible-arrays and the deprecation of zero-lenght and one-element arrays. Thanks -- Gustavo Documentation/process/deprecated.rst | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/Documentation/process/deprecated.rst b/Documentation/process/deprecated.rst index 652e2aa02a66c..0b7b37718bf96 100644 --- a/Documentation/process/deprecated.rst +++ b/Documentation/process/deprecated.rst @@ -85,6 +85,17 @@ Instead, use the helper:: header = kzalloc(struct_size(header, item, count), GFP_KERNEL); +NOTE: You might want to use the following form in case the trailing array +is a one-element array, as unlike zero-length arrays and flexible-array +members, `one-element arrays occupy at least as much space as a single +object of the type <https://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html>`_, +hence they contribute to the size of the enclosing structure:: + + header = kzalloc(struct_size(header, item, count - 1), GFP_KERNEL); + +It's also worth noting that one-element arrays --together with zero-length +arrays-- will soon be completely removed from the codebase and deprecated. + See array_size(), array3_size(), and struct_size(), for more details as well as the related check_add_overflow() and check_mul_overflow() family of functions. -- 2.27.0 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] docs: deprecated.rst: Add note to the use of struct_size() helper 2020-06-04 17:21 [PATCH] docs: deprecated.rst: Add note to the use of struct_size() helper Gustavo A. R. Silva @ 2020-06-04 17:49 ` Kees Cook 2020-06-04 18:21 ` Gustavo A. R. Silva 0 siblings, 1 reply; 6+ messages in thread From: Kees Cook @ 2020-06-04 17:49 UTC (permalink / raw) To: Gustavo A. R. Silva Cc: Jonathan Corbet, linux-doc, linux-kernel, Gustavo A. R. Silva On Thu, Jun 04, 2020 at 12:21:38PM -0500, Gustavo A. R. Silva wrote: > Add a note to educate people about the proper use of struct_size() when > the trailing array in the enclosing structure is a one-element array. > > Zero-length and one-element arrays will soon be removed from the kernel, > but in the meantime, it's worth letting people know how to correctly > use struct_size() together with such constructs. > > Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org> > --- > > Kees, > > This is not substitute for the patch I'll write about flexible-arrays > and the deprecation of zero-lenght and one-element arrays. Hm, hm. I think I'd rather just get the 0/1-array docs written, since that will mean this paragraph isn't needed at all. (Or rather, it can be modified to say "if you're using struct_size() on a 1-array, stop using a 1-array, see [link]". If someone is going to switch around their code, they need to switch to flex at the same time, IMO. -Kees > > Thanks > -- > Gustavo > > Documentation/process/deprecated.rst | 11 +++++++++++ > 1 file changed, 11 insertions(+) > > diff --git a/Documentation/process/deprecated.rst b/Documentation/process/deprecated.rst > index 652e2aa02a66c..0b7b37718bf96 100644 > --- a/Documentation/process/deprecated.rst > +++ b/Documentation/process/deprecated.rst > @@ -85,6 +85,17 @@ Instead, use the helper:: > > header = kzalloc(struct_size(header, item, count), GFP_KERNEL); > > +NOTE: You might want to use the following form in case the trailing array > +is a one-element array, as unlike zero-length arrays and flexible-array > +members, `one-element arrays occupy at least as much space as a single > +object of the type <https://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html>`_, > +hence they contribute to the size of the enclosing structure:: > + > + header = kzalloc(struct_size(header, item, count - 1), GFP_KERNEL); > + > +It's also worth noting that one-element arrays --together with zero-length > +arrays-- will soon be completely removed from the codebase and deprecated. > + > See array_size(), array3_size(), and struct_size(), > for more details as well as the related check_add_overflow() and > check_mul_overflow() family of functions. > -- > 2.27.0 > -- Kees Cook ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] docs: deprecated.rst: Add note to the use of struct_size() helper 2020-06-04 17:49 ` Kees Cook @ 2020-06-04 18:21 ` Gustavo A. R. Silva 2020-06-04 20:25 ` Kees Cook 0 siblings, 1 reply; 6+ messages in thread From: Gustavo A. R. Silva @ 2020-06-04 18:21 UTC (permalink / raw) To: Kees Cook; +Cc: Jonathan Corbet, linux-doc, linux-kernel, Gustavo A. R. Silva On Thu, Jun 04, 2020 at 10:49:19AM -0700, Kees Cook wrote: > On Thu, Jun 04, 2020 at 12:21:38PM -0500, Gustavo A. R. Silva wrote: > > Add a note to educate people about the proper use of struct_size() when > > the trailing array in the enclosing structure is a one-element array. > > > > Zero-length and one-element arrays will soon be removed from the kernel, > > but in the meantime, it's worth letting people know how to correctly > > use struct_size() together with such constructs. > > > > Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org> > > --- > > > > Kees, > > > > This is not substitute for the patch I'll write about flexible-arrays > > and the deprecation of zero-lenght and one-element arrays. > > Hm, hm. I think I'd rather just get the 0/1-array docs written, since > that will mean this paragraph isn't needed at all. (Or rather, it can be Yeah. My reasoning for is that it will take a while --at least one development cycle more-- to completely get rid of all the 0/1-arrays. Also, this was motivated by the following comments from Christian König: "May I suggest that we add a section how to correctly do this to Documentation/process/coding-style.rst or similar document? I've seen a bunch of different approaches and some even doesn't work with some gcc versions and result in a broken binary."[1] > modified to say "if you're using struct_size() on a 1-array, stop using > a 1-array, see [link]". If someone is going to switch around their code, > they need to switch to flex at the same time, IMO. > I agree with this. I can add the comments in quotes you suggest to this patch. But I think we can add this note while I continue working on the flexible-array conversions. Once that work is complete, I can go back and update the documentation. :) What do you think? Thanks -- Gustavo [1] https://lore.kernel.org/lkml/1065d63e-7959-e4b4-af4e-70607ba92296@amd.com/ > > > > Documentation/process/deprecated.rst | 11 +++++++++++ > > 1 file changed, 11 insertions(+) > > > > diff --git a/Documentation/process/deprecated.rst b/Documentation/process/deprecated.rst > > index 652e2aa02a66c..0b7b37718bf96 100644 > > --- a/Documentation/process/deprecated.rst > > +++ b/Documentation/process/deprecated.rst > > @@ -85,6 +85,17 @@ Instead, use the helper:: > > > > header = kzalloc(struct_size(header, item, count), GFP_KERNEL); > > > > +NOTE: You might want to use the following form in case the trailing array > > +is a one-element array, as unlike zero-length arrays and flexible-array > > +members, `one-element arrays occupy at least as much space as a single > > +object of the type <https://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html>`_, > > +hence they contribute to the size of the enclosing structure:: > > + > > + header = kzalloc(struct_size(header, item, count - 1), GFP_KERNEL); > > + > > +It's also worth noting that one-element arrays --together with zero-length > > +arrays-- will soon be completely removed from the codebase and deprecated. > > + > > See array_size(), array3_size(), and struct_size(), > > for more details as well as the related check_add_overflow() and > > check_mul_overflow() family of functions. > > -- > > 2.27.0 > > > > -- > Kees Cook ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] docs: deprecated.rst: Add note to the use of struct_size() helper 2020-06-04 18:21 ` Gustavo A. R. Silva @ 2020-06-04 20:25 ` Kees Cook 2020-06-04 21:19 ` Gustavo A. R. Silva 0 siblings, 1 reply; 6+ messages in thread From: Kees Cook @ 2020-06-04 20:25 UTC (permalink / raw) To: Gustavo A. R. Silva Cc: Jonathan Corbet, linux-doc, linux-kernel, Gustavo A. R. Silva On Thu, Jun 04, 2020 at 01:21:23PM -0500, Gustavo A. R. Silva wrote: > Yeah. My reasoning for is that it will take a while --at least one > development cycle more-- to completely get rid of all the 0/1-arrays. Right -- but we need a place to point people when we tell them "please don't use 0-byte and 1-byte arrays", and the deprecated.rst is the place for that. Having it in deprecated.rst once they're all gone only serves to explain why various compiler flags are enabled, etc. But while they're being removed, it serves as a single place to document the issue (as in, much of the flex-array patch commit log "boilerplate" can actually be repeated in deprecated.rst. > But I think we can add this note while I continue working on the flexible-array > conversions. Once that work is complete, I can go back and update the > documentation. :) > > What do you think? I think we need to document it at the beginning of the work (and I should have asked for this earlier). So let's just add a new section on dynamic array usage, etc. It can include a note about struct_size() as an example for why 1-byte arrays are so weird. :) -- Kees Cook ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] docs: deprecated.rst: Add note to the use of struct_size() helper 2020-06-04 20:25 ` Kees Cook @ 2020-06-04 21:19 ` Gustavo A. R. Silva 2020-06-04 21:33 ` Kees Cook 0 siblings, 1 reply; 6+ messages in thread From: Gustavo A. R. Silva @ 2020-06-04 21:19 UTC (permalink / raw) To: Kees Cook; +Cc: Jonathan Corbet, linux-doc, linux-kernel, Gustavo A. R. Silva On Thu, Jun 04, 2020 at 01:25:26PM -0700, Kees Cook wrote: > On Thu, Jun 04, 2020 at 01:21:23PM -0500, Gustavo A. R. Silva wrote: > > Yeah. My reasoning for is that it will take a while --at least one > > development cycle more-- to completely get rid of all the 0/1-arrays. > > Right -- but we need a place to point people when we tell them "please > don't use 0-byte and 1-byte arrays", and the deprecated.rst is the place > for that. > > Having it in deprecated.rst once they're all gone only serves to explain > why various compiler flags are enabled, etc. But while they're being > removed, it serves as a single place to document the issue (as in, much > of the flex-array patch commit log "boilerplate" can actually be > repeated in deprecated.rst. > > > But I think we can add this note while I continue working on the flexible-array > > conversions. Once that work is complete, I can go back and update the > > documentation. :) > > > > What do you think? > > I think we need to document it at the beginning of the work (and I > should have asked for this earlier). So let's just add a new section on > dynamic array usage, etc. It can include a note about struct_size() as > an example for why 1-byte arrays are so weird. :) > Got ya. :) One last thing... I was thinking on adding such section (dynamic array usage) to coding-style.rst, explaining how to use struct_size() and transform the different open-coded versions we currently have in the kernel, e.g. I have seen people use offsetof() --and sometimes open-coded versions of sizeof_field()-- and its open-coded version to do arithmetic in allocator arguments. Thanks -- Gustavo ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] docs: deprecated.rst: Add note to the use of struct_size() helper 2020-06-04 21:19 ` Gustavo A. R. Silva @ 2020-06-04 21:33 ` Kees Cook 0 siblings, 0 replies; 6+ messages in thread From: Kees Cook @ 2020-06-04 21:33 UTC (permalink / raw) To: Gustavo A. R. Silva Cc: Jonathan Corbet, linux-doc, linux-kernel, Gustavo A. R. Silva On Thu, Jun 04, 2020 at 04:19:03PM -0500, Gustavo A. R. Silva wrote: > On Thu, Jun 04, 2020 at 01:25:26PM -0700, Kees Cook wrote: > > On Thu, Jun 04, 2020 at 01:21:23PM -0500, Gustavo A. R. Silva wrote: > > > Yeah. My reasoning for is that it will take a while --at least one > > > development cycle more-- to completely get rid of all the 0/1-arrays. > > > > Right -- but we need a place to point people when we tell them "please > > don't use 0-byte and 1-byte arrays", and the deprecated.rst is the place > > for that. > > > > Having it in deprecated.rst once they're all gone only serves to explain > > why various compiler flags are enabled, etc. But while they're being > > removed, it serves as a single place to document the issue (as in, much > > of the flex-array patch commit log "boilerplate" can actually be > > repeated in deprecated.rst. > > > > > But I think we can add this note while I continue working on the flexible-array > > > conversions. Once that work is complete, I can go back and update the > > > documentation. :) > > > > > > What do you think? > > > > I think we need to document it at the beginning of the work (and I > > should have asked for this earlier). So let's just add a new section on > > dynamic array usage, etc. It can include a note about struct_size() as > > an example for why 1-byte arrays are so weird. :) > > > > Got ya. :) > > One last thing... I was thinking on adding such section (dynamic array > usage) to coding-style.rst, explaining how to use struct_size() and > transform the different open-coded versions we currently have in the > kernel, e.g. I have seen people use offsetof() --and sometimes open-coded > versions of sizeof_field()-- and its open-coded version to do arithmetic > in allocator arguments. Yeah, that sounds good to me! -Kees > > Thanks > -- > Gustavo > -- Kees Cook ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-06-04 21:33 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-06-04 17:21 [PATCH] docs: deprecated.rst: Add note to the use of struct_size() helper Gustavo A. R. Silva 2020-06-04 17:49 ` Kees Cook 2020-06-04 18:21 ` Gustavo A. R. Silva 2020-06-04 20:25 ` Kees Cook 2020-06-04 21:19 ` Gustavo A. R. Silva 2020-06-04 21:33 ` Kees Cook
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).