linux-kernel - Re: [PATCH 1/2] lib/string_helpers: Introduce strsplit

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:   Tue, 9 Aug 2022 17:23:06 +0200
From:   Andy Shevchenko <andy.shevchenko@...il.com>
To:     Cezary Rojewski <cezary.rojewski@...el.com>
Cc:     Péter Ujfalusi <peter.ujfalusi@...ux.intel.com>,
        Andy Shevchenko <andy@...nel.org>,
        Mark Brown <broonie@...nel.org>,
        ALSA Development Mailing List <alsa-devel@...a-project.org>,
        Takashi Iwai <tiwai@...e.com>,
        Jaroslav Kysela <perex@...ex.cz>,
        amadeuszx.slawinski@...ux.intel.com,
        Pierre-Louis Bossart <pierre-louis.bossart@...ux.intel.com>,
        Hans de Goede <hdegoede@...hat.com>,
        Ranjani Sridharan <ranjani.sridharan@...ux.intel.com>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Liam Girdwood <lgirdwood@...il.com>,
        Kai Vehmanen <kai.vehmanen@...ux.intel.com>,
        Bard Liao <yung-chuan.liao@...ux.intel.com>
Subject: Re: [PATCH 1/2] lib/string_helpers: Introduce strsplit_u32()

On Tue, Aug 9, 2022 at 11:55 AM Cezary Rojewski
<cezary.rojewski@...el.com> wrote:
> On 2022-07-12 4:24 PM, Andy Shevchenko wrote:

...

> I've spent some time analyzing possible utilization of _Generic() in
> context of get_options() but in my opinion get_range() complicates
> things enough that get_range() and get_option() would basically need a
> copy per type.

Thanks for keeping us updated.

> If Linux kernel guarantees that sizeof(int), sizeof(unsigned int),
> sizeof(s32) and sizeof(u32) are all equal (given the currently supported
> arch set), then indeed modifying get_options() may not be necessary.
> This plus shamelessly casting (u32 *) to (int *) of course.

I think as long as Linux kernel states that it requires (at least)
32-bit architecture to run, we are fine. I have heard of course about
a funny project of running Linux on 8-bit microcontrollers, but it's
such a fun niche, which by the way uses emulation without changing
actual 32-bit code, that I won't even talk about.

> What's left to do is the __user helper function. What I have in mind is:
>
> int tokenize_user_input(const char __user *from, size_t count, loff_t
> *ppos, int **tkns)
> {
>         int *ints, nints;
>         char *buf;
>         int ret;
>
>         buf = kmalloc(count + 1, GFP_KERNEL);
>         if (!buf)
>                 return -ENOMEM;
>
>         ret = simple_write_to_buffer(buf, count, ppos, from, count);
>         if (ret != count) {
>                 ret = (ret < 0) ? ret : -EIO;
>                 goto free_buf;
>         }
>
>         buf[count] = '\0';

I guess this may be simplified with memdup_user(). Otherwise it looks like that.

>         get_options(buf, 0, &nints);

(You don't use ppos here, so it's pointless to use
simple_write_to_buffer(), right? I have noticed this pattern in SOF
code, which might be simplified the same way as I suggested above)

>         if (!nints) {
>                 ret = -ENOENT;
>                 goto free_buf;
>         }
>
>         ints = kcalloc(nints + 1, sizeof(*ints), GFP_KERNEL);
>         if (!ints) {
>                 ret = -ENOMEM;
>                 goto free_buf;
>         }
>
>         get_options(buf, nints + 1, ints);
>         *tkns = ints;
>         ret = 0;
>
> free_buf:
>         kfree(buf);
>         return ret;
> }

...

> as a part of fs/libfs.c not lib/cmdline.c. Is such approach acceptable?

I think so.

-- 
With Best Regards,
Andy Shevchenko