2018-05-02 07:51:40

by Daniel Vetter

[permalink] [raw]
Subject: [PATCH] doc: botching-up-ioctls: Make it clearer why structs must be padded

This came up in discussions when reviewing drm patches.

Cc: Eric Anholt <[email protected]>
Cc: [email protected]
Cc: Jonathan Corbet <[email protected]>
Signed-off-by: Daniel Vetter <[email protected]>

--

Aside: I wonder whether we shouldn't move this to some other place and
rst-ify it? Any good suggestions?
-Daniel
---
Documentation/ioctl/botching-up-ioctls.txt | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/Documentation/ioctl/botching-up-ioctls.txt b/Documentation/ioctl/botching-up-ioctls.txt
index d02cfb48901c..883fb034bd04 100644
--- a/Documentation/ioctl/botching-up-ioctls.txt
+++ b/Documentation/ioctl/botching-up-ioctls.txt
@@ -73,7 +73,9 @@ will have a second iteration or at least an extension for any given interface.
future extensions is going right down the gutters since someone will submit
an ioctl struct with random stack garbage in the yet unused parts. Which
then bakes in the ABI that those fields can never be used for anything else
- but garbage.
+ but garbage. This is also the reason why you must explicitly pad all
+ structures, even if you never use them in an array - the padding the compiler
+ might insert could contain garbage.

* Have simple testcases for all of the above.

--
2.17.0



2018-05-02 16:57:41

by Eric Anholt

[permalink] [raw]
Subject: Re: [PATCH] doc: botching-up-ioctls: Make it clearer why structs must be padded

Daniel Vetter <[email protected]> writes:

> This came up in discussions when reviewing drm patches.
>
> Cc: Eric Anholt <[email protected]>
> Cc: [email protected]
> Cc: Jonathan Corbet <[email protected]>
> Signed-off-by: Daniel Vetter <[email protected]>
>
> --
>
> Aside: I wonder whether we shouldn't move this to some other place and
> rst-ify it? Any good suggestions?
> -Daniel
> ---
> Documentation/ioctl/botching-up-ioctls.txt | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/ioctl/botching-up-ioctls.txt b/Documentation/ioctl/botching-up-ioctls.txt
> index d02cfb48901c..883fb034bd04 100644
> --- a/Documentation/ioctl/botching-up-ioctls.txt
> +++ b/Documentation/ioctl/botching-up-ioctls.txt
> @@ -73,7 +73,9 @@ will have a second iteration or at least an extension for any given interface.
> future extensions is going right down the gutters since someone will submit
> an ioctl struct with random stack garbage in the yet unused parts. Which
> then bakes in the ABI that those fields can never be used for anything else
> - but garbage.
> + but garbage. This is also the reason why you must explicitly pad all
> + structures, even if you never use them in an array - the padding the compiler
> + might insert could contain garbage.

I hadn't realized that we had this document in git, or I probably would
have written this patch. I think this makes it clear enough how I got
vc4 and v3d wrong. Thanks!

Reviewed-by: Eric Anholt <[email protected]>


Attachments:
signature.asc (847.00 B)

2018-05-08 14:53:46

by Jonathan Corbet

[permalink] [raw]
Subject: Re: [PATCH] doc: botching-up-ioctls: Make it clearer why structs must be padded

On Wed, 2 May 2018 09:51:06 +0200
Daniel Vetter <[email protected]> wrote:

> This came up in discussions when reviewing drm patches.

Applied, thanks.

> Aside: I wonder whether we shouldn't move this to some other place and
> rst-ify it? Any good suggestions?

For the moment, probably in Documentation/process, next to
volatile-considered-harmful.rst and such. Even better, of course, would
be to have some nice document on designing user-space APIs in general...
one can always dream ... :)

Thanks,

jon