linux-kernel - Re: [PATCH v1 1/2] mmc-utils: Use memcpy instead of strncpy

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:   Mon, 15 Nov 2021 17:07:45 +0530
From:   Ajay Garg <ajaygargnsit@...il.com>
To:     Bean Huo <huobean@...il.com>
Cc:     ulf.hansson@...aro.org, adrian.hunter@...el.com,
        linux-mmc@...r.kernel.org, linux-kernel@...r.kernel.org,
        beanhuo@...ron.com
Subject: Re: [PATCH v1 1/2] mmc-utils: Use memcpy instead of strncpy

> I don't understand how above memcpy() overflow/underflow/out-of-bounds?
> would you please provide more specific reason?
>
> memcpy(lbuf, (char*)&ext_csd[EXT_CSD_FIRMWARE_VERSION], 8);
>
> here lbuf is a char array lbuf[10], and ext_csd is a __u8 array, __u8
> ext_csd[512].

Ok, in the given information parameters, it might work fine at runtime.

But still, using 8 as the magic number makes the code illegible for a
third-party. Plus the code is also unoptimised, eg :

i)
If ext = "abc", then we need to copy (3 + 1) bytes.
However, currently memcpy would copy (8 + 1) bytes.

ii)
If ext = "abcdefghijklmnopqrst", then we need to copy (9 + 1) bytes.
However, currently memcpy would copy (8 + 1) bytes.


> Yes, but please remember that mmc-utils is mainly used for embedded
> platforms, they are not easy/inconvenient to update to the latest
> library to support these two APIs(strlcpy needs libbsd-dev, and strscpy
> needs some one else.).  If we use strlcpy or strscpy, mmc-utils will
> not be portable. Do you know any other API that can be used and make
> code more portable and simpler?
>

Hmm, you can always start adding code locally in your codebase.
Anyways, if you *must* use only "already available code", snprintf is
an alternative.

snprintf(lbuf, sizeof(lbuf), (char*)&ext_csd[EXT_CSD_FIRMWARE_VERSION]);