linux-kernel - Re: [PATCH] usbip: fix warning in vhci_hcd_probe/lockdep_init

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 for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <8167945c-36b5-1797-b220-54428d4c7869@osg.samsung.com>
Date:   Wed, 7 Dec 2016 19:51:11 -0700
From:   Shuah Khan <shuahkh@....samsung.com>
To:     fx IWATA NOBUO <Nobuo.Iwata@...ixerox.co.jp>,
        Andrey Konovalov <andreyknvl@...gle.com>
Cc:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Valentina Manea <valentina.manea.m@...il.com>,
        Shuah Khan <shuah@...nel.org>,
        "linux-usb@...r.kernel.org" <linux-usb@...r.kernel.org>,
        LKML <linux-kernel@...r.kernel.org>,
        Shuah Khan <shuah@...nel.org>,
        Shuah Khan <shuahkh@....samsung.com>
Subject: Re: [PATCH] usbip: fix warning in vhci_hcd_probe/lockdep_init_map

Hi Nobuo.iwata,

I noticed that in many places sysfs_attr_init() is called
before populating the fields such as name etc. However, I
don't think the order matters.

sysfs_attr_init() doesn't depend on name or any other fields
being set:

#define sysfs_attr_init(attr)                           \
do {                                                    \
        static struct lock_class_key __key;             \
                                                        \
        (attr)->key = &__key;                           \
} while (0)

Are you concerned about something else?

thanks,
-- Shuah


On 12/07/2016 07:15 PM, fx IWATA NOBUO wrote:
> Dear Shuah,
> 
> I'm afraid there's one thing I have concerned.
> 
> I think it's better to move sysfs_attr_init() before
> status->attr.attr.name = status->name;
> 
> Best Regards,
> 
> nobuo.iwata
> //
>> -----Original Message-----
>> From: linux-usb-owner@...r.kernel.org
>> [mailto:linux-usb-owner@...r.kernel.org] On Behalf Of Andrey Konovalov
>> Sent: Tuesday, December 06, 2016 9:07 PM
>> To: Shuah Khan
>> Cc: Greg Kroah-Hartman; Valentina Manea; Shuah Khan;
>> linux-usb@...r.kernel.org; LKML
>> Subject: Re: [PATCH] usbip: fix warning in vhci_hcd_probe/lockdep_init_map
>>
>> On Mon, Dec 5, 2016 at 9:00 PM, Shuah Khan <shuahkh@....samsung.com> wrote:
>>> Hi Andrey,
>>>
>>> On 12/05/2016 12:56 PM, Shuah Khan wrote:
>>>> vhci_hcd calls sysfs_create_group() with dynamically allocated sysfs
>>>> attributes triggering the lock-class key not persistent warning. Call
>>>> sysfs_attr_init() for dynamically allocated sysfs attributes to fix it.
>>>>
>>>> vhci_hcd vhci_hcd: USB/IP Virtual Host Controller vhci_hcd vhci_hcd:
>>>> new USB bus registered, assigned bus number 2
>>>> BUG: key ffff88006a7e8d18 not in .data!
>>>> ------------[ cut here ]------------
>>>> WARNING: CPU: 0 PID: 1 at kernel/locking/lockdep.c:3131
>>>> lockdep_init_map+0x60c/0x770
>>>> DEBUG_LOCKS_WARN_ON(1)[    1.567044] Modules linked in:
>>>> CPU: 0 PID: 1 Comm: swapper/0 Not tainted 4.9.0-rc7+ #58 Hardware
>>>> name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
>>>>  ffff88006bce6eb8 ffffffff81f96c8a ffffffff00000a02 1ffff1000d79cd6a
>>>>  ffffed000d79cd62 000000046bce6ed8 0000000041b58ab3 ffffffff8598af40
>>>>  ffffffff81f969f8 0000000000000000 0000000041b58ab3 0000000000000200
>>>> Call Trace:
>>>>  [<     inline     >] __dump_stack lib/dump_stack.c:15
>>>>  [<ffffffff81f96c8a>] dump_stack+0x292/0x398 lib/dump_stack.c:51
>>>> [<ffffffff812b808f>] __warn+0x19f/0x1e0 kernel/panic.c:550
>>>> [<ffffffff812b8195>] warn_slowpath_fmt+0xc5/0x110 kernel/panic.c:565
>>>> [<ffffffff813f3efc>] lockdep_init_map+0x60c/0x770
>>>> kernel/locking/lockdep.c:3131  [<ffffffff819e43d4>]
>>>> __kernfs_create_file+0x114/0x2a0 fs/kernfs/file.c:954
>> [<ffffffff819e68f5>] sysfs_add_file_mode_ns+0x225/0x520
>> fs/sysfs/file.c:305
>>>>  [<     inline     >] create_files fs/sysfs/group.c:64
>>>>  [<ffffffff819e8a89>] internal_create_group+0x239/0x8f0
>>>> fs/sysfs/group.c:134  [<ffffffff819e915f>]
>>>> sysfs_create_group+0x1f/0x30 fs/sysfs/group.c:156
>>>> [<ffffffff8323de24>] vhci_start+0x5b4/0x7a0
>>>> drivers/usb/usbip/vhci_hcd.c:978  [<ffffffff82c907ca>]
>>>> usb_add_hcd+0x8da/0x1c60 drivers/usb/core/hcd.c:2867
>>>> [<ffffffff8323bc57>] vhci_hcd_probe+0x97/0x130
>>>> drivers/usb/usbip/vhci_hcd.c:1103
>>>>  ---
>>>>  ---
>>>> ---[ end trace c33c7b202cf3aac8 ]---
>>>>
>>>> Signed-off-by: Shuah Khan <shuahkh@....samsung.com>
>>>> Reported-by: Andrey Konovalov <andreyknvl@...gle.com>
>>>
>>> Here is the fix. Fixed the warning I reproduced on my system.
>>> Let me know if it works for you.
>>
>> Hi Shuah,
>>
>> This fixes the warning I've been seeing.
>>
>> Thanks!
>>
>>>
>>> thanks,
>>> -- Shuah
>>>
>>>> ---
>>>>  drivers/usb/usbip/vhci_sysfs.c | 1 +
>>>>  1 file changed, 1 insertion(+)
>>>>
>>>> diff --git a/drivers/usb/usbip/vhci_sysfs.c
>>>> b/drivers/usb/usbip/vhci_sysfs.c index c404017..b96e5b1 100644
>>>> --- a/drivers/usb/usbip/vhci_sysfs.c
>>>> +++ b/drivers/usb/usbip/vhci_sysfs.c
>>>> @@ -361,6 +361,7 @@ static void set_status_attr(int id)
>>>>       status->attr.attr.name = status->name;
>>>>       status->attr.attr.mode = S_IRUGO;
>>>>       status->attr.show = status_show;
>>>> +     sysfs_attr_init(&status->attr.attr);
>>>>  }
>>>>
>>>>  static int init_status_attrs(void)
>>>>
>>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-usb" in
>> the body of a message to majordomo@...r.kernel.org More majordomo info at
>> http://vger.kernel.org/majordomo-info.html