linux-kernel - Re: [PATCH 1/2] device.h: pack struct dev_links

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20190227092318.GK4747@localhost>
Date:   Wed, 27 Feb 2019 10:23:18 +0100
From:   Johan Hovold <johan@...nel.org>
To:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Cc:     linux-kernel@...r.kernel.org,
        "Rafael J. Wysocki" <rafael.j.wysocki@...el.com>
Subject: Re: [PATCH 1/2] device.h: pack struct dev_links_info

On Tue, Feb 26, 2019 at 03:41:07PM +0100, Greg Kroah-Hartman wrote:
> The dev_links_info structure has 4 bytes of padding at the end of it
> when embedded in struct device (which is the only place it lives).  To
> help reduce the size of struct device pack this structure so we can take
> advantage of the hole with later structure reorganizations.
> 
> Cc: "Rafael J. Wysocki" <rafael.j.wysocki@...el.com>
> Signed-off-by: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
> ---
>  include/linux/device.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/include/linux/device.h b/include/linux/device.h
> index 6cb4640b6160..b63165276a09 100644
> --- a/include/linux/device.h
> +++ b/include/linux/device.h
> @@ -884,7 +884,7 @@ struct dev_links_info {
>  	struct list_head suppliers;
>  	struct list_head consumers;
>  	enum dl_dev_state status;
> -};
> +} __packed;

This seems like a bad idea. You're changing the alignment of these
fields to one byte, something which may cause the compiler to generate
less efficient code to deal with unaligned accesses (even if they happen
to currently be naturally aligned in struct device).

I don't think we should mess with __packed unless for things that
actually require it (e.g. data going on to the wire) even if it means
wasting 4 bytes on 64-bit archs.

Johan