linux-kernel - Re: [PATCH 01/12] mfd: Eberspaecher Flexcard PMC II Carrier Board support

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]

Date:   Wed, 14 Dec 2016 09:38:54 +0100
From:   Arnd Bergmann <arnd@...db.de>
To:     Holger Dengler <dengler@...utronix.de>
Cc:     Lee Jones <lee.jones@...aro.org>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Vinod Koul <vinod.koul@...el.com>,
        linux-kernel@...r.kernel.org, dmaengine@...r.kernel.org,
        Thomas Gleixner <tglx@...utronix.de>,
        Sebastian Siewior <bigeasy@...utronix.de>,
        Juergen Bubeck <bubeck@...ug.com>,
        Peter Mahler <mahler@...ug.com>,
        Benedikt Spranger <b.spranger@...utronix.de>
Subject: Re: [PATCH 01/12] mfd: Eberspaecher Flexcard PMC II Carrier Board support

On Wednesday, December 14, 2016 1:11:42 AM CET Holger Dengler wrote:
>
> diff --git a/include/uapi/linux/flexcard.h b/include/uapi/linux/flexcard.h
> new file mode 100644
> index 0000000..4e9f07b4
> --- /dev/null
> +++ b/include/uapi/linux/flexcard.h
> @@ -0,0 +1,64 @@

Why is this exported to user space?

> +
> +#include <linux/types.h>
> +
> +struct fc_version {
> +	__u8	dev;
> +	__u8	min;
> +	__u8	maj;
> +	__u8	reserved;
> +} __packed;

The __packed attribute is redundant here as all members
are just one byte anyway.

> +/* PCI BAR 0: Flexcard configuration */
> +struct fc_bar0_conf {
> +	__u32 r1;			/* 000 */
> +	struct fc_version fc_fw_ver;	/* 004 */
> +	struct fc_version fc_hw_ver;	/* 008 */
> +	__u32 r2[3];			/* 00c */
> +	__u64 fc_sn;			/* 018 */
> +	__u32 fc_uid;			/* 020 */
> +	__u32 r3[7];			/* 024 */
> +	__u32 fc_lic[6];		/* 040 */
> +	__u32 fc_slic[6];		/* 058 */
> +	__u32 trig_ctrl1;		/* 070 */
> +	__u32 r4;			/* 074 */
> +	__u32 trig_ctrl2;		/* 078 */
> +	__u32 r5[22];			/* 07c */
> +	__u32 amreg;			/* 0d4 */
> +	__u32 tiny_stat;		/* 0d8 */
> +	__u32 r6[5];			/* 0dc */
> +	__u32 can_dat_cnt;		/* 0f0 */
> +	__u32 can_err_cnt;		/* 0f4 */
> +	__u32 fc_data_cnt;		/* 0f8 */
> +	__u32 r7;			/* 0fc */
> +	__u32 fc_rocr;			/* 100 */
> +	__u32 r8;			/* 104 */
> +	__u32 pg_ctrl;			/* 108 */
> +	__u32 pg_term;			/* 10c */
> +	__u32 r9;			/* 110 */
> +	__u32 irs;			/* 114 */
> +	__u32 fr_tx_cnt;		/* 118 */
> +	__u32 irc;			/* 11c */
> +	__u64 pcnt;			/* 120 */
> +	__u32 r10;			/* 128 */
> +	__u32 nmv_cnt;			/* 12c */
> +	__u32 info_cnt;			/* 130 */
> +	__u32 stat_trg_cnt;		/* 134 */
> +	__u32 r11;			/* 138 */
> +	__u32 fr_rx_cnt;		/* 13c */
> +} __packed;

Here the __packed attribute is probably harmful, it prevents you
from accessing the members using 32-bit sized accesses and forces
bytewise accesses on some architectures, which tends to do the
wrong thing on MMIO.

	Arnd