linux-kernel - RE: [PATCH] mmc: truncate quirks' oemid to 8 bits

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]

Message-ID: <DM6PR04MB657596E19EF3D7976197660EFCDDA@DM6PR04MB6575.namprd04.prod.outlook.com>
Date:   Thu, 26 Oct 2023 10:16:53 +0000
From:   Avri Altman <Avri.Altman@....com>
To:     Dominique Martinet <dominique.martinet@...ark-techno.com>
CC:     Dominique Martinet <asmadeus@...ewreck.org>,
        "linux-mmc@...r.kernel.org" <linux-mmc@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "stable@...r.kernel.org" <stable@...r.kernel.org>,
        Ulf Hansson <ulf.hansson@...aro.org>,
        Alex Fetters <Alex.Fetters@...min.com>
Subject: RE: [PATCH] mmc: truncate quirks' oemid to 8 bits

> We now only capture 8 bits for oemid in card->cid.oemid, so quirks that
> were filling up the full 16 bits up till now would no longer apply.
> 
> Work around the problem by only checking for the bottom 8 bits when
> checking if quirks should be applied
> 
> Fixes: 84ee19bffc93 ("mmc: core: Capture correct oemid-bits for eMMC
> cards")
> Link: https://lkml.kernel.org/r/ZToJsSLHr8RnuTHz@codewreck.org
> Signed-off-by: Dominique Martinet <dominique.martinet@...ark-
> techno.com>
> Cc: stable@...r.kernel.org
> Cc: Avri Altman <avri.altman@....com>
> Cc: Ulf Hansson <ulf.hansson@...aro.org>
> Cc: Alex Fetters <Alex.Fetters@...min.com>
Reviewed-by: Avri Altman <avri.altman@....com>

> ---
> Notes:
>  - mmc_fixup_device() was rewritten in 5.17, so older stable kernels
>    will need a separate patch... I suppose I can send it to stable
>    after this is merged if we go this way
>  - struct mmc_cid's and mmc_fixup's oemid fields are unsigned shorts,
>    we probably just want to make them unsigned char instead in which
>    case we don't need that check anymore?
>    But it's kind of nice to have a wider type so CID_OEMID_ANY can never
>    be a match.... Which unfortunately my patch makes moot as
>    ((unsigned short)-1) & 0xff will be 0xff which can match anything...
>  - this could also be worked around in the _FIXUP_EXT macro that builds
>    the fixup structs, but we're getting ugly here... Or we can just go
>    for the big boom and try to fix all MMC_FIXUP() users in tree and
>    call it a day, but that'll also be fun to backport.
To me, your fix is clean, elegant and does the job.
I would let the quirk owners to fix that hard-coded bogus oemid - should they choose to.
I guess Sandisk would need to do that as well.

Thanks,
Avri

> 
>  drivers/mmc/core/quirks.h | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mmc/core/quirks.h b/drivers/mmc/core/quirks.h
> index 32b64b564fb1..27e0349e176d 100644
> --- a/drivers/mmc/core/quirks.h
> +++ b/drivers/mmc/core/quirks.h
> @@ -211,8 +211,9 @@ static inline void mmc_fixup_device(struct
> mmc_card *card,
>                 if (f->manfid != CID_MANFID_ANY &&
>                     f->manfid != card->cid.manfid)
>                         continue;
> +               /* Only the bottom 8bits are valid in JESD84-B51 */
>                 if (f->oemid != CID_OEMID_ANY &&
> -                   f->oemid != card->cid.oemid)
> +                    (f->oemid & 0xff) != (card->cid.oemid & 0xff))
>                         continue;
>                 if (f->name != CID_NAME_ANY &&
>                     strncmp(f->name, card->cid.prod_name,
> --
> 2.39.2
>