Re: VAIO EEPROM support in at24

[Bartosz Golaszewski <brgl@bgdev.pl>]

wt., 17 mar 2020 o 15:14 Jean Delvare <jdelvare@suse.de> napisał(a):
>
> Hi all,
>
> As the legacy eeprom driver is being phased out, I am reviewing all its
> use cases to ensure that the at24 driver will be a suitable replacement.
>
> One issue I have found is the handling of specific EEPROMs found on the
> SMBus of some Sony VAIO laptops. The legacy eeprom driver would expose
> them to user-space, read-only for all users. It would also recognize
> them as VAIO EEPROMs and would hide some bytes from non-root users
> because these bytes contain the BIOS password in a lightly encoded form.
>
> In order to keep the same level of functionality, we would have to offer
> the same with the at24 driver. At this time, the user has to
> instantiate a "24c02" device manually from user-space. By default this
> device has permissions 600, which is insufficient for users, and
> dangerous for root, so a quick chmod 444 is needed.
>
> Without the password issue, I would just document the procedure and
> live with it. However in order to protect the password from being read
> by random users, the driver itself needs to know that it is dealing
> with a specific type of EEPROM. It seems that we need to introduce a
> new device flavor to the at24 driver for this purpose.
>
> I see that we already have a number of specific flags (AT24_FLAG_SERIAL
> and AT24_FLAG_MAC) so I suppose we could add something similar for
> these VAIO EEPROMs. Something like:
>
> /* Some Sony VAIO laptops have a 24c02 at 0x57 with product info */
> AT24_CHIP_DATA(at24_data_sony_vaio, 2048 / 8,
>         AT24_FLAG_READONLY | AT24_FLAG_IRUGO | AT24_FLAG_SONY_VAIO);
>
> Then I suppose it's only a matter of conditionally zeroing a selected
> range in at24_read() before it returns, to hide the password from
> non-root users.
>
> Before I start implementing the idea above, I would like to know if
> anyone objects to it, or has a better idea?
>

Sounds good to me in general but isn't it something we could
generalize a bit more?

For instance we could make at24_chip_data struct look something like this:

struct at24_chip_data {
    u32 byte_len;
    u8 flags;
    struct resource masked;
};

And we could introduce a new macro called AT24_CHIP_DATA_MASKED that
would automacially set the AT24_FLAG_MASKED_RANGE flag and take
another argument that would contain the address and size of the masked
register range (we'd put it into the "masked" resource)?

Other ideas are welcome too. I just think that making it
SONY_VAIO-specific may be a bit limiting in the future.

Best regards,
Bartosz Golaszewski