linux-kernel - Re: [PATCH v4] usb: core: Add "quirks" parameter for usbcore

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:   Thu, 1 Mar 2018 00:01:40 +0800
From:   Kai Heng Feng <kai.heng.feng@...onical.com>
To:     Matthew Wilcox <willy@...radead.org>
Cc:     gregkh@...uxfoundation.org, oneukum@...e.com, corbet@....net,
        linux-doc@...r.kernel.org, linux-kernel@...r.kernel.org,
        linux-usb@...r.kernel.org
Subject: Re: [PATCH v4] usb: core: Add "quirks" parameter for usbcore



> On 28 Feb 2018, at 10:47 PM, Matthew Wilcox <willy@...radead.org> wrote:
>
> On Mon, Feb 26, 2018 at 11:04:57PM +0800, Kai-Heng Feng wrote:
>> +static char quirks_param[128];
>> +module_param_string(quirks, quirks_param, sizeof(quirks_param), 0644);
>> +MODULE_PARM_DESC(quirks, "Add/modify USB quirks by specifying  
>> quirks=vendorID:productID:quirks");
>> +
>> +static char quirks_param_orig[128];
>
>> +static u32 usb_detect_dynamic_quirks(struct usb_device *udev)
>> +{
>> +	u16 vid = le16_to_cpu(udev->descriptor.idVendor);
>> +	u16 pid = le16_to_cpu(udev->descriptor.idProduct);
>> +	struct quirk_entry *quirk;
>> +
>> +	mutex_lock(&quirk_mutex);
>> +	if (strcmp(quirks_param, quirks_param_orig) != 0) {
>> +		strcpy(quirks_param_orig, quirks_param);
>
> What happens if the user is writing to quirks_param at the same time
> that you memcpy it?
>
> I think you're going about this wrong by trying to use the
> module_param_string machinery.  You should be using module_param_cb()
> to build the quirks list when the user writes it (and then translate
> back into a string when the user wants to read from it.

Thanks! module_param_cb() is exactly what I want.
I’ll use it in next version.

> Also, you won't need to use a linked list for this; you can just allocate
> an array of quirks.
>

I use linked list because the total quirks number is known after the entire  
string gets parsed.
Do you suggest that I should just alloc a predefined number (like 16) quirk  
entries, instead of doing it dynamically?

Kai-Heng