| 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
| ||
|
Message-ID:
<PNZPR01MB4478E080F6EDAFC6C34A08A6B8C12@PNZPR01MB4478.INDPRD01.PROD.OUTLOOK.COM>
Date: Sun, 23 Feb 2025 06:39:15 +0000
From: Aditya Garg <gargaditya08@...e.com>
To: "andriy.shevchenko@...ux.intel.com" <andriy.shevchenko@...ux.intel.com>
CC: "pmladek@...e.com" <pmladek@...e.com>, "rostedt@...dmis.org"
<rostedt@...dmis.org>, "linux@...musvillemoes.dk" <linux@...musvillemoes.dk>,
"senozhatsky@...omium.org" <senozhatsky@...omium.org>, "corbet@....net"
<corbet@....net>, "maarten.lankhorst@...ux.intel.com"
<maarten.lankhorst@...ux.intel.com>, "mripard@...nel.org"
<mripard@...nel.org>, "tzimmermann@...e.de" <tzimmermann@...e.de>,
"airlied@...il.com" <airlied@...il.com>, "simona@...ll.ch" <simona@...ll.ch>,
"akpm@...ux-foundation.org" <akpm@...ux-foundation.org>, "apw@...onical.com"
<apw@...onical.com>, "joe@...ches.com" <joe@...ches.com>,
"dwaipayanray1@...il.com" <dwaipayanray1@...il.com>,
"lukas.bulwahn@...il.com" <lukas.bulwahn@...il.com>,
"sumit.semwal@...aro.org" <sumit.semwal@...aro.org>,
"christian.koenig@....com" <christian.koenig@....com>, "kekrby@...il.com"
<kekrby@...il.com>, "admin@...eit.net" <admin@...eit.net>, Orlando
Chamberlain <orlandoch.dev@...il.com>, "evepolonium@...il.com"
<evepolonium@...il.com>, "linux-doc@...r.kernel.org"
<linux-doc@...r.kernel.org>, "linux-kernel@...r.kernel.org"
<linux-kernel@...r.kernel.org>, "dri-devel@...ts.freedesktop.org"
<dri-devel@...ts.freedesktop.org>, "linux-media@...r.kernel.org"
<linux-media@...r.kernel.org>, "linaro-mm-sig@...ts.linaro.org"
<linaro-mm-sig@...ts.linaro.org>, Hector Martin <marcan@...can.st>,
"linux@...linux.org.uk" <linux@...linux.org.uk>, "asahi@...ts.linux.dev"
<asahi@...ts.linux.dev>, Sven Peter <sven@...npeter.dev>, Janne Grunau
<j@...nau.net>
Subject: Re: [PATCH v2 2/3] lib/vsprintf: Add support for generic FOURCCs by
extending %p4cc
> On 22 Feb 2025, at 5:41 PM, Aditya Garg <gargaditya08@...e.com> wrote:
>
>
>
>> On 21 Feb 2025, at 8:57 PM, andriy.shevchenko@...ux.intel.com wrote:
>>> On Thu, Feb 20, 2025 at 04:39:23PM +0000, Aditya Garg wrote:
>>> From: Hector Martin <marcan@...can.st>
>>> %p4cc is designed for DRM/V4L2 FOURCCs with their specific quirks, but
>>> it's useful to be able to print generic 4-character codes formatted as
>>> an integer. Extend it to add format specifiers for printing generic
>>> 32-bit FOURCCs with various endian semantics:
>>> %p4ch Host-endian
>>> %p4cl Little-endian
>>> %p4cb Big-endian
>>> %p4cr Reverse-endian
>>> The endianness determines how bytes are interpreted as a u32, and the
>>> FOURCC is then always printed MSByte-first (this is the opposite of
>>> V4L/DRM FOURCCs). This covers most practical cases, e.g. %p4cr would
>>> allow printing LSByte-first FOURCCs stored in host endian order
>>> (other than the hex form being in character order, not the integer
>>> value).
>> ...
>>> orig = get_unaligned(fourcc);
>>> - val = orig & ~BIT(31);
>>> + switch (fmt[2]) {
>>> + case 'h':
>>> + val = orig;
>>> + break;
>>> + case 'r':
>>> + orig = swab32(orig);
>>> + val = orig;
>>> + break;
>>> + case 'l':
>>> + orig = le32_to_cpu(orig);
>>> + val = orig;
>>> + break;
>>> + case 'b':
>>> + orig = be32_to_cpu(orig);
>> I do not see that orig is a union of different types. Have you run sparse?
>> It will definitely complain on this code.
>
> After messing around with this, what I’ve noticed is that orig and val used in this struct should be u32. Now in case of little endian and big endian, that things are messy. The original code by Hector was using le32_to_cpu on orig, which itself is declared as a u32 here (maybe was done with the intention to convert le32 orig to u32 orig?).
>
> Anyways, what I have done is that:
>
> 1. Declare new variable, orig_le which is __le32.
> 2. Instead of doing orig = le32_to_cpu(orig); , we can do orig_le = cpu_to_le32(orig). This fixes the sparse warning: cast to restricted __le32
> 3. Now the original code was intending to use val=orig=le32_to_cpu(orig) at the bottom part of this struct. Those parts also require val and orig to be u32. For that, we are now using le32_to_cpu(orig_le). Since val is same as orig, in case these cases, instead of making a val_le, I’ve simply used orig_le there as well.
>
> Similar changes done for big endian.
>
> So, the struct looks like this now:
>
> static noinline_for_stack
> char *fourcc_string(char *buf, char *end, const u32 *fourcc,
> struct printf_spec spec, const char *fmt)
> {
> char output[sizeof("0123 little-endian (0x01234567)")];
> char *p = output;
> unsigned int i;
> unsigned char c;
> bool pixel_fmt = false;
> u32 orig, val;
> __le32 orig_le;
> __be32 orig_be;
>
> if (fmt[1] != 'c')
> return error_string(buf, end, "(%p4?)", spec);
>
> if (check_pointer(&buf, end, fourcc, spec))
> return buf;
>
> orig = get_unaligned(fourcc);
> switch (fmt[2]) {
> case 'h':
> val = orig;
> break;
> case 'r':
> orig = swab32(orig);
> val = orig;
> break;
> case 'l':
> orig_le = cpu_to_le32(orig);
> break;
> case 'b':
> orig_be = cpu_to_be32(orig);
> break;
> case 'c':
> /* Pixel formats are printed LSB-first */
> val = swab32(orig & ~BIT(31));
> pixel_fmt = true;
> break;
> default:
> return error_string(buf, end, "(%p4?)", spec);
> }
>
> for (i = 0; i < sizeof(u32); i++) {
> switch (fmt[2]) {
> case 'h':
> case 'r':
> case 'c':
> c = val >> ((3 - i) * 8);
> break;
> case 'l':
> c = le32_to_cpu(orig_le) >> ((3 - i) * 8);
> break;
> case 'b':
> c = be32_to_cpu(orig_be) >> ((3 - i) * 8);
> break;
> }
>
> /* Print non-control ASCII characters as-is, dot otherwise */
> *p++ = isascii(c) && isprint(c) ? c : '.';
> }
>
> if (pixel_fmt) {
> *p++ = ' ';
> strcpy(p, orig & BIT(31) ? "big-endian" : "little-endian");
> p += strlen(p);
> }
>
> *p++ = ' ';
> *p++ = '(';
>
> switch (fmt[2]) {
> case 'h':
> case 'r':
> case 'c':
> p = special_hex_number(p, output + sizeof(output) - 2, orig, sizeof(u32));
> break;
> case 'l':
> p = special_hex_number(p, output + sizeof(output) - 2, le32_to_cpu(orig_le), sizeof(u32));
> break;
> case 'b':
> p = special_hex_number(p, output + sizeof(output) - 2, be32_to_cpu(orig_be), sizeof(u32));
> break;
> }
>
> *p++ = ')';
> *p = '\0';
>
> return string(buf, end, output, spec);
> }
>
> Andy, could you verify this?
Looking at the header files, it looks like doing cpu_to_le32 on that variable and doing le32_to_cpu will actually reverse the order twice, on big endian systems, thus technically all way would not swap the order at all.
I'm not really sure how to manage the sparse warnings here.
Powered by blists - more mailing lists