linux-kernel - Re: [PATCH v2 1/2] selftests/resctrl: Fix schemata write error check

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: <gp6qu6zqa7h23ddqfff4mwigh7b5tzfsfkqlrpu7xz4yydk4ck@pgxge6ftz7we>
Date:   Thu, 31 Aug 2023 10:06:44 +0200
From:   Maciej Wieczór-Retman 
        <maciej.wieczor-retman@...el.com>
To:     Reinette Chatre <reinette.chatre@...el.com>
CC:     <linux-kernel@...r.kernel.org>, <linux-kselftest@...r.kernel.org>,
        <shuah@...nel.org>, <fenghua.yu@...el.com>,
        <ilpo.jarvinen@...ux.intel.com>
Subject: Re: [PATCH v2 1/2] selftests/resctrl: Fix schemata write error check

Hi,
thanks for the comments!

On 2023-08-30 at 13:49:18 -0700, Reinette Chatre wrote:
>Hi Maciej,
>
>On 8/28/2023 2:56 AM, Wieczor-Retman, Maciej wrote:
>> Writing bitmasks to the schemata can fail when the bitmask doesn't
>> adhere to some constraints defined by what a particular CPU supports.
>> Some example of constraints are max length or having contiguous bits.
>> The driver should properly return errors when any rule concerning
>> bitmask format is broken.
>> 
>> Resctrl FS returns error codes from fprintf() only when fclose() is
>> called. Current error checking scheme allows invalid bitmasks to be
>> written into schemata file and the selftest doesn't notice because the
>> fclose() error code isn't checked.
>> 
>> Substitute fopen(), flose() and fprintf() with open(), close() and
>> write() to avoid error code buffering between fprintf() and fclose().
>> 
>> Add newline to the end of the schema string so it satisfies rdt
>> schemata writing requirements.
>
>I am not sure how to interpret the above because existing code already
>adds a newline to the end of the schema when the buffer is written to

Okay, true. I meant I moved it a few lines back but there is no real
change there. I'll just remove this paragraph.

>the schemata file. Also please use "resctrl schemata" since RDT is
>Intel specific and does not use schemata terminology.

Thank you, I'll change it.

>> 
>> Remove newline character from the schemat string after writing it to
>> the schemata file so it prints correctly before function return.
>
>schemat -> "schema" or "schemata"?

I'll stick with schema as that's the variable name, thanks for finding
this typo.

>> Pass the string generated with strerror() to the "reason" buffer so
>> the error message is more verbose. Extend "reason" buffer so it can hold
>> longer messages.
>> 
>> Changelog v2:
>> - Rewrite patch message.
>> - Double "reason" buffer size to fit longer error explanation.
>> - Redo file interactions with syscalls instead of stdio functions.
>> 
>
>Please place the above "Changelog v2" snippet below the "---" lines below.
>This is text that should not end up in the kernel log.

Yes, I realized I made this mistake a few hours after sending the patch.
I'll correct it and make sure to double check it before sending next time.

>> Signed-off-by: Wieczor-Retman, Maciej <maciej.wieczor-retman@...el.com>
>> ---
>
>(list of changes should go here)
>
>Reinette
>

-- 
Kind regards
Maciej Wieczór-Retman