linux-kernel - Re: [PATCH] i2c: piix4: Avoid race conditions with IMC

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:   Thu, 12 Jan 2017 10:40:06 +0100
From:   Jean Delvare <jdelvare@...e.de>
To:     Andy Shevchenko <andy.shevchenko@...il.com>
Cc:     Ricardo Ribalda Delgado <ricardo.ribalda@...il.com>,
        Wolfram Sang <wsa@...-dreams.de>, linux-i2c@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH] i2c: piix4: Avoid race conditions with IMC

On Wed, 11 Jan 2017 03:49:21 +0200, Andy Shevchenko wrote:
> On Tue, Jan 10, 2017 at 2:16 PM, Ricardo Ribalda Delgado
> > --- a/drivers/i2c/busses/i2c-piix4.c
> > +++ b/drivers/i2c/busses/i2c-piix4.c
> > @@ -585,9 +585,28 @@ static s32 piix4_access_sb800(struct i2c_adapter *adap, u16 addr,
> >                  u8 command, int size, union i2c_smbus_data *data)
> >  {
> >         struct i2c_piix4_adapdata *adapdata = i2c_get_adapdata(adap);
> > +       unsigned short piix4_smba = adapdata->smba;
> >         u8 smba_en_lo;
> >         u8 port;
> >         int retval;
> > +       int timeout = 0;
> > +       int smbslvcnt;
> 
> Keep them just after your another added variable.

FWIW, I don't think this makes sense as a general rule. I'd rather have
the variables in an order which makes sense (for human readers or for
stack size optimization - unless gcc does it for us?), rather than
always adding at the same place. Is there a rationale for doing that? I
don't think shrinking the patch size is good enough a reason.

-- 
Jean Delvare
SUSE L3 Support