netdev - Re: [PATCH 1/2] net: dsa: ksz9477: add I2C managed mode 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:   Sun, 16 Dec 2018 09:35:08 +0100
From:   Sergio Paracuellos <sergio.paracuellos@...il.com>
To:     andrew@...n.ch
Cc:     Woojung.Huh@...rochip.com, UNGLinuxDriver@...rochip.com,
        vivien.didelot@...oirfairelinux.com, f.fainelli@...il.com,
        driverdev-devel@...uxdriverproject.org, davem@...emloft.net,
        netdev@...r.kernel.org,
        "open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" 
        <devicetree@...r.kernel.org>
Subject: Re: [PATCH 1/2] net: dsa: ksz9477: add I2C managed mode support

On Sun, Dec 16, 2018 at 9:15 AM Andrew Lunn <andrew@...n.ch> wrote:
>
> On Sun, Dec 16, 2018 at 08:57:40AM +0100, Sergio Paracuellos wrote:
> > +static int ksz_i2c_read_reg(struct i2c_client *client, u32 reg, u8 *val,
> > +                         unsigned int len)
> > +{
> > +     struct i2c_adapter *adapter = client->adapter;
> > +     struct i2c_msg msg[2];
> > +     u8 txd[2];
>

Hi Andrew,

> Hi Sergio
>
> I'm not sure that having the TX buffer on the stack is safe. If the
> i2c bus master is using DMA, you then DMA from the stack, which some
> architectures memory models do no allow. You have to use memory which
> comes from an alloc function.
>
> > +     int ret;
> > +
> > +     txd[0] = (u8)(reg >> 8);
> > +     txd[1] = (u8)reg;
> > +
> > +     msg[0].addr = client->addr;
> > +     msg[0].flags = 0;
> > +     msg[0].len = 2;
> > +     msg[0].buf = txd;
> > +
> > +     msg[1].addr = client->addr;
> > +     msg[1].flags = I2C_M_RD;
> > +     msg[1].len = len;
> > +     msg[1].buf = val;
>
> You potentially have the same issue with val.
>

I'll change these two to to get memory from kernel allocators instead
of using the stack. Thanks for let me know this.

>     Andrew

Best regards,
    Sergio Paracuellos