netdev - Re: [patch v2] net/usb: remove err() messages in few drivers

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: <208cbae30901131622heccfee6uf312022ba1d4fb28@mail.gmail.com>
Date:	Wed, 14 Jan 2009 03:22:26 +0300
From:	"Alexey Klimov" <klimov.linux@...il.com>
To:	"Oliver Neukum" <oliver@...kum.org>
Cc:	"David Brownell" <david-b@...bell.net>, davem@...emloft.net,
	"Greg KH" <greg@...ah.com>, netdev@...r.kernel.org,
	linux-usb@...r.kernel.org
Subject: Re: [patch v2] net/usb: remove err() messages in few drivers

Hello, all

On Tue, Jan 13, 2009 at 2:07 PM, Oliver Neukum <oliver@...kum.org> wrote:
> Am Tuesday 13 January 2009 11:08:27 schrieb Alexey Klimov:
>> On Tue, Jan 13, 2009 at 12:44 PM, David Brownell <david-b@...bell.net> wrote:
>> > On Tuesday 13 January 2009, Oliver Neukum wrote:
>> >> > Ok, i'll reformat patch. Is it okay if i use kaweth->dev->dev,
>> >> > kaweth->net->dev and intf->dev ? Or switch to kaweth->net->dev
>> >> > (instead of dev->dev)?
>> >>
>> >> kaweth in kaweth_disconnect() frees the network device. Therefore
>> >> you cannot use it. You'd access freed memory. Pick one of the others
>> >> and stay with it.
>> >
>> > intf->dev is going to be "obviously correct", the others aren't.
>> >
>>
>> Is this patch touch  kaweth_disconnect() ? I see dev_warn and dev_info
>> there. No dev_err.
>
> No, it does not. But you need to make sure that the devices you refer to
> in debug messages are always valid. Functions that you do touch may
> run after disconnect() has run. A debug message that oopses does no good.
> If you cannot make sure you pass a valid device to dev_err/info don't use
> them. It is as simple as that.

Yes, that's obviously right. I'll review and check patch again.
Thank you for your advices.


On Tue, Jan 13, 2009 at 12:46 PM, David Brownell <david-b@...bell.net> wrote:
> On Monday 12 January 2009, Alexey Klimov wrote:
>> -                               err("submit(rx_urb) status %d", res);
>> +                               dev_err(&catc->usbdev->dev, "submit(rx_urb) status %d\n", res);
>
>
> That's the same as urb->dev though.
>
> Use the interface passed into probe():  dev_err(&intf->dev, ...) etc

Well, in few v4l-dvb drivers successfully used &radio->usbdev->dev in
debug messages and that doesn't look wrong.

Hmm, i see that struct usb_interface passed to probe function for
example, and here in catc_irq_done struct urb passed, and then we have
struct catc. So, messages based on catc->usbdev->dev. I can switch to
&catc->netdev->dev when it's safe to do, right?
I didn't see way we can use &intv->dev in such functions. Am i wrong ?
In probe functions &intf->dev is right.

Btw, in patch "USB: remove info() macro from usb network drivers"

http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=880c9c66a60c0aa4fb4dac2da9679da5f8f41903

few messages based on &urb->dev->dev.

-- 
Best regards, Klimov Alexey
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html