| 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:
<PN3PR01MB95978AB9BE01393753278A5EB8D02@PN3PR01MB9597.INDPRD01.PROD.OUTLOOK.COM>
Date: Wed, 12 Mar 2025 12:07:50 +0000
From: Aditya Garg <gargaditya08@...e.com>
To: Thomas Zimmermann <tzimmermann@...e.de>
CC: Aun-Ali Zaidi <admin@...eit.net>, Maxime Ripard <mripard@...nel.org>,
"airlied@...hat.com" <airlied@...hat.com>, Simona Vetter <simona@...ll.ch>,
"andriy.shevchenko@...ux.intel.com" <andriy.shevchenko@...ux.intel.com>, Petr
Mladek <pmladek@...e.com>, Steven Rostedt <rostedt@...dmis.org>, Rasmus
Villemoes <linux@...musvillemoes.dk>, "senozhatsky@...omium.org"
<senozhatsky@...omium.org>, Jonathan Corbet <corbet@....net>, Andrew Morton
<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>, Linux Kernel Mailing List
<linux-kernel@...r.kernel.org>, "dri-devel@...ts.freedesktop.org"
<dri-devel@...ts.freedesktop.org>, "linux-doc@...r.kernel.org"
<linux-doc@...r.kernel.org>, Hector Martin <marcan@...can.st>, Sven Peter
<sven@...npeter.dev>, "asahi@...ts.linux.dev" <asahi@...ts.linux.dev>
Subject: Re: [PATCH 1/2] lib/vsprintf: Add support for generic FourCCs by
extending %p4cc
> On 12 Mar 2025, at 5:33 PM, Aditya Garg <gargaditya08@...e.com> wrote:
>
>
>
>> On 12 Mar 2025, at 5:29 PM, Thomas Zimmermann <tzimmermann@...e.de> wrote:
>>
>> Hi
>>
>>>> Am 12.03.25 um 12:49 schrieb Aditya Garg:
>>>
>>>>> On 12 Mar 2025, at 5:16 PM, Thomas Zimmermann <tzimmermann@...e.de> wrote:
>>>>
>>>> Hi
>>>>
>>>>> Am 12.03.25 um 10:05 schrieb Aditya Garg:
>>>>> 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 byte order
>>>>> %p4cn Network byte order
>>>>> %p4cl Little-endian
>>>>> %p4cb Big-endian
>>>> That looks like someone trying to be too clever for their own good. Just my 2 cts.
>>> I don't understand what you are trying to say. Anyways, I thought it's obvious, but Petr's Ack is still left and thus cannot be merged into DRM for now unless he says so in this thread.
>>
>> I'm trying to say that the author of this patch found the %p4cc functionality and over-generalized the feature. Source code should express the idea of what it's doing in clear terms. %p4ch somehow doesn't do that for me. Printing 4 bytes in various orders without context seems arbitrary and confusing.
>>
>> (I don't really have a say here. I'm just asking to reconsider this change.)
>
> Ah I see. I'll checkout the macros you sent. The Asahi Linux SMC drivers would need these as well, so I'll probably first wait for the vsprintf maintainers and also Asahi Linux maintainers for their views.
>>
You also might have noticed that the FourCCs actually make sense in this driver. Eg: REDY, CLRD stand for APPLETBDRM_MSG_SIGNAL_READINESS and APPLETBDRM_MSG_CLEAR_DISPLAY respectively.
>> Best regards
>> Thomas
>>
>>>> Best regards
>>>> Thomas
>>>>
>>>>> 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. %p4cn would
>>>>> allow printing LSByte-first FourCCs stored in host endian order
>>>>> (other than the hex form being in character order, not the integer
>>>>> value).
>>>>>
>>>>> Acked-by: Rasmus Villemoes <linux@...musvillemoes.dk>
>>>>> Reviewed-by: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
>>>>> Signed-off-by: Hector Martin <marcan@...can.st>
>>>>> Signed-off-by: Aditya Garg <gargaditya08@...e.com>
>>>>> ---
>>>>> Documentation/core-api/printk-formats.rst | 32 +++++++++++++++++++
>>>>> lib/test_printf.c | 39 +++++++++++++++++++----
>>>>> lib/vsprintf.c | 35 ++++++++++++++++----
>>>>> scripts/checkpatch.pl | 2 +-
>>>>> 4 files changed, 94 insertions(+), 14 deletions(-)
>>>>>
>>>>> diff --git a/Documentation/core-api/printk-formats.rst b/Documentation/core-api/printk-formats.rst
>>>>> index ecccc0473..bd420e8aa 100644
>>>>> --- a/Documentation/core-api/printk-formats.rst
>>>>> +++ b/Documentation/core-api/printk-formats.rst
>>>>> @@ -648,6 +648,38 @@ Examples::
>>>>> %p4cc Y10 little-endian (0x20303159)
>>>>> %p4cc NV12 big-endian (0xb231564e)
>>>>> +Generic FourCC code
>>>>> +-------------------
>>>>> +
>>>>> +::
>>>>> + %p4c[hnlb] gP00 (0x67503030)
>>>>> +
>>>>> +Print a generic FourCC code, as both ASCII characters and its numerical
>>>>> +value as hexadecimal.
>>>>> +
>>>>> +The generic FourCC code is always printed in the big-endian format,
>>>>> +the most significant byte first. This is the opposite of V4L/DRM FourCCs.
>>>>> +
>>>>> +The additional ``h``, ``n``, ``l``, and ``b`` specifiers define what
>>>>> +endianness is used to load the stored bytes. The data might be interpreted
>>>>> +using the host byte order, network byte order, little-endian, or big-endian.
>>>>> +
>>>>> +Passed by reference.
>>>>> +
>>>>> +Examples for a little-endian machine, given &(u32)0x67503030::
>>>>> +
>>>>> + %p4ch gP00 (0x67503030)
>>>>> + %p4cn 00Pg (0x30305067)
>>>>> + %p4cl gP00 (0x67503030)
>>>>> + %p4cb 00Pg (0x30305067)
>>>>> +
>>>>> +Examples for a big-endian machine, given &(u32)0x67503030::
>>>>> +
>>>>> + %p4ch gP00 (0x67503030)
>>>>> + %p4cn 00Pg (0x30305067)
>>>>> + %p4cl 00Pg (0x30305067)
>>>>> + %p4cb gP00 (0x67503030)
>>>>> +
>>>>> Rust
>>>>> ----
>>>>> diff --git a/lib/test_printf.c b/lib/test_printf.c
>>>>> index 59dbe4f9a..b9e8afc01 100644
>>>>> --- a/lib/test_printf.c
>>>>> +++ b/lib/test_printf.c
>>>>> @@ -776,21 +776,46 @@ static void __init fwnode_pointer(void)
>>>>> software_node_unregister_node_group(group);
>>>>> }
>>>>> +struct fourcc_struct {
>>>>> + u32 code;
>>>>> + const char *str;
>>>>> +};
>>>>> +
>>>>> +static void __init fourcc_pointer_test(const struct fourcc_struct *fc, size_t n,
>>>>> + const char *fmt)
>>>>> +{
>>>>> + size_t i;
>>>>> +
>>>>> + for (i = 0; i < n; i++)
>>>>> + test(fc[i].str, fmt, &fc[i].code);
>>>>> +}
>>>>> +
>>>>> static void __init fourcc_pointer(void)
>>>>> {
>>>>> - struct {
>>>>> - u32 code;
>>>>> - char *str;
>>>>> - } const try[] = {
>>>>> + static const struct fourcc_struct try_cc[] = {
>>>>> { 0x3231564e, "NV12 little-endian (0x3231564e)", },
>>>>> { 0xb231564e, "NV12 big-endian (0xb231564e)", },
>>>>> { 0x10111213, ".... little-endian (0x10111213)", },
>>>>> { 0x20303159, "Y10 little-endian (0x20303159)", },
>>>>> };
>>>>> - unsigned int i;
>>>>> + static const struct fourcc_struct try_ch[] = {
>>>>> + { 0x41424344, "ABCD (0x41424344)", },
>>>>> + };
>>>>> + static const struct fourcc_struct try_cn[] = {
>>>>> + { 0x41424344, "DCBA (0x44434241)", },
>>>>> + };
>>>>> + static const struct fourcc_struct try_cl[] = {
>>>>> + { (__force u32)cpu_to_le32(0x41424344), "ABCD (0x41424344)", },
>>>>> + };
>>>>> + static const struct fourcc_struct try_cb[] = {
>>>>> + { (__force u32)cpu_to_be32(0x41424344), "ABCD (0x41424344)", },
>>>>> + };
>>>>> - for (i = 0; i < ARRAY_SIZE(try); i++)
>>>>> - test(try[i].str, "%p4cc", &try[i].code);
>>>>> + fourcc_pointer_test(try_cc, ARRAY_SIZE(try_cc), "%p4cc");
>>>>> + fourcc_pointer_test(try_ch, ARRAY_SIZE(try_ch), "%p4ch");
>>>>> + fourcc_pointer_test(try_cn, ARRAY_SIZE(try_cn), "%p4cn");
>>>>> + fourcc_pointer_test(try_cl, ARRAY_SIZE(try_cl), "%p4cl");
>>>>> + fourcc_pointer_test(try_cb, ARRAY_SIZE(try_cb), "%p4cb");
>>>>> }
>>>>> static void __init
>>>>> diff --git a/lib/vsprintf.c b/lib/vsprintf.c
>>>>> index 56fe96319..56511a994 100644
>>>>> --- a/lib/vsprintf.c
>>>>> +++ b/lib/vsprintf.c
>>>>> @@ -1781,27 +1781,50 @@ char *fourcc_string(char *buf, char *end, const u32 *fourcc,
>>>>> char output[sizeof("0123 little-endian (0x01234567)")];
>>>>> char *p = output;
>>>>> unsigned int i;
>>>>> + bool pixel_fmt = false;
>>>>> u32 orig, val;
>>>>> - if (fmt[1] != 'c' || fmt[2] != 'c')
>>>>> + if (fmt[1] != 'c')
>>>>> return error_string(buf, end, "(%p4?)", spec);
>>>>> if (check_pointer(&buf, end, fourcc, spec))
>>>>> return buf;
>>>>> orig = get_unaligned(fourcc);
>>>>> - val = orig & ~BIT(31);
>>>>> + switch (fmt[2]) {
>>>>> + case 'h':
>>>>> + break;
>>>>> + case 'n':
>>>>> + orig = swab32(orig);
>>>>> + break;
>>>>> + case 'l':
>>>>> + orig = (__force u32)cpu_to_le32(orig);
>>>>> + break;
>>>>> + case 'b':
>>>>> + orig = (__force u32)cpu_to_be32(orig);
>>>>> + break;
>>>>> + case 'c':
>>>>> + /* Pixel formats are printed LSB-first */
>>>>> + pixel_fmt = true;
>>>>> + break;
>>>>> + default:
>>>>> + return error_string(buf, end, "(%p4?)", spec);
>>>>> + }
>>>>> +
>>>>> + val = pixel_fmt ? swab32(orig & ~BIT(31)) : orig;
>>>>> for (i = 0; i < sizeof(u32); i++) {
>>>>> - unsigned char c = val >> (i * 8);
>>>>> + unsigned char c = val >> ((3 - i) * 8);
>>>>> /* Print non-control ASCII characters as-is, dot otherwise */
>>>>> *p++ = isascii(c) && isprint(c) ? c : '.';
>>>>> }
>>>>> - *p++ = ' ';
>>>>> - strcpy(p, orig & BIT(31) ? "big-endian" : "little-endian");
>>>>> - p += strlen(p);
>>>>> + if (pixel_fmt) {
>>>>> + *p++ = ' ';
>>>>> + strcpy(p, orig & BIT(31) ? "big-endian" : "little-endian");
>>>>> + p += strlen(p);
>>>>> + }
>>>>> *p++ = ' ';
>>>>> *p++ = '(';
>>>>> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
>>>>> index 7b28ad331..5595a0898 100755
>>>>> --- a/scripts/checkpatch.pl
>>>>> +++ b/scripts/checkpatch.pl
>>>>> @@ -6904,7 +6904,7 @@ sub process {
>>>>> ($extension eq "f" &&
>>>>> defined $qualifier && $qualifier !~ /^w/) ||
>>>>> ($extension eq "4" &&
>>>>> - defined $qualifier && $qualifier !~ /^cc/)) {
>>>>> + defined $qualifier && $qualifier !~ /^c[hnlbc]/)) {
>>>>> $bad_specifier = $specifier;
>>>>> last;
>>>>> }
>>>> --
>>>> --
>>>> Thomas Zimmermann
>>>> Graphics Driver Developer
>>>> SUSE Software Solutions Germany GmbH
>>>> Frankenstrasse 146, 90461 Nuernberg, Germany
>>>> GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
>>>> HRB 36809 (AG Nuernberg)
>>>>
>>
>> --
>> --
>> Thomas Zimmermann
>> Graphics Driver Developer
>> SUSE Software Solutions Germany GmbH
>> Frankenstrasse 146, 90461 Nuernberg, Germany
>> GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
>> HRB 36809 (AG Nuernberg)
>>
Powered by blists - more mailing lists