Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
References: <20200317151409.7940926c@endymion>
In-Reply-To: <20200317151409.7940926c@endymion>
From:   Bartosz Golaszewski <brgl@bgdev.pl>
Date:   Tue, 17 Mar 2020 15:32:56 +0100
Message-ID: <CAMRc=Mdoh5Sk3iS_CO4+++SG2jJOy1qrG4q2zOzbeYYMdJR0VA@mail.gmail.com>
Subject: Re: VAIO EEPROM support in at24
To:     Jean Delvare <jdelvare@suse.de>
Cc:     LKML <linux-kernel@vger.kernel.org>,
        Linux I2C <linux-i2c@vger.kernel.org>,
        Bartosz Golaszewski <bgolaszewski@baylibre.com>,
        Arnd Bergmann <arnd@arndb.de>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

wt., 17 mar 2020 o 15:14 Jean Delvare <jdelvare@suse.de> napisa=C5=82(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