linux-kernel - Re: [PATCH] Export device_add_attributes() so drivers can use it.

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:	Wed, 18 Feb 2009 20:14:19 +0100
From:	Kay Sievers <kay.sievers@...y.org>
To:	Greg KH <gregkh@...e.de>
Cc:	Grant Likely <grant.likely@...retlab.ca>,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] Export device_add_attributes() so drivers can use it.

On Wed, Feb 18, 2009 at 19:39, Greg KH <gregkh@...e.de> wrote:
> On Wed, Feb 18, 2009 at 07:32:19PM +0100, Kay Sievers wrote:
>> On Wed, Feb 18, 2009 at 18:51, Grant Likely <grant.likely@...retlab.ca> wrote:
>> > On Wed, Feb 18, 2009 at 9:49 AM, Kay Sievers <kay.sievers@...y.org> wrote:
>> >> On Wed, Feb 18, 2009 at 17:34, Grant Likely <grant.likely@...retlab.ca> wrote:
>> >>> On Wed, Feb 18, 2009 at 8:53 AM, Kay Sievers <kay.sievers@...y.org> wrote:
>> >>>> On Wed, Feb 18, 2009 at 16:48, Grant Likely <grant.likely@...retlab.ca> wrote:
>> >>>>> On Wed, Feb 18, 2009 at 8:45 AM, Kay Sievers <kay.sievers@...y.org> wrote:
>> >>>>>> On Wed, Feb 18, 2009 at 16:29, Greg KH <gregkh@...e.de> wrote:
>> >>>>>>> On Wed, Feb 18, 2009 at 08:11:34AM -0700, Grant Likely wrote:
>> >>>>>>>> From: Grant Likely <grant.likely@...retlab.ca>
>> >>>>>>>>
>> >>>>>>>> I find myself using the pattern of device_add_attributes() and
>> >>>>>>>> device_remove_attributes() frequently in my drivers.  Rather than
>> >>>>>>>> reinventing the wheel every time, I'm floating this patch to export
>> >>>>>>>> the symbols to see how it is received.  If this looks okay then I'll
>> >>>>>>>> rework my drivers and post additional patches to use these functions.
>> >>>>>>>
>> >>>>>>> No objection from me, as long as the symbols are EXPORT_SYMBOL_GPL(),
>> >>>>>>> like the rest of the driver core.  Is that ok with you?
>> >>>>>>
>> >>>>>> These functions used outside the core create attributes after the
>> >>>>>> uevent is sent, and userspace will not see these files at event time.
>> >>>>>> This is in most cases a pretty broken behavior. Is that the expected
>> >>>>>> behavior in your drivers?
>> >>>>>
>> >>>>> ??? I don't follow what you mean.
>> >>>>>
>> >>>>> I'm using these functions to allow the driver to add device attribs;
>> >>>>> primarily for debugging knobs and controls.  Userspace will see the
>> >>>>> files after the driver is bound to the device.  The uevent doesn't
>> >>>>> really come into play.
>> >>>>
>> >>>> Sure, they do. Many things expect all files which are visible at the
>> >>>> device to be readable also at event time. That's the whole way udev
>> >>>> and device property matching works. There are only a few exceptions
>> >>>> where creating files at a device later, after it is registered with
>> >>>> the core, is not a bug.
>> >>>
>> >>> Let me make sure I understand you...
>> >>>
>> >>> Is it a bug for a device driver to call
>> >>> device_create_file()/device_remove_file() at probe time?  For example,
>> >>> if I have a data capture device which is probed via the platform bus,
>> >>> is it okay for the .probe() function for the driver to use
>> >>> device_create_file() to add a 'rate_statistics' file which dumps out
>> >>> some data rate statistics in ASCII form?
>> >>>
>> >>> I was under the impression that
>> >>> device_create_file()/device_remove_file() were okay to use at probe
>> >>> time.  device_add_attributes()/device_remove_attributes() are only
>> >>> wrappers around device_create_file()/device_remove_file() with error
>> >>> checking and unwinding when things go wrong.
>> >>>
>> >>> Am I incorrect here?
>> >>
>> >> You are probing an existing "struct device", and then create
>> >> attributes at this device when the probe succeeds?
>> >
>> > Yes
>> >
>> >> If yes, why don't
>> >> you create a new child "struct device" with your functionality and add
>> >> the attributes there?
>> >
>> > Mostly because I have no need another full struct device, and none of
>> > the files that I'm adding have any bearing on udev.  They are debug
>> > and statistical files for an embedded system that are used by the
>> > developer.  I don't want application code depending on them and I'm
>> > not interested in having them advertised.
>>
>> That's what debugfs is for.
>
> Kay is right, use debugfs for stuff like this.  Not sysfs.
>
> So sorry, I'm not going to apply the patch, and you should change your
> code to not open-code this as well.

You can probably use an attribute group instead, if you want to create
a bunch of attributes with error handling. If the group has a name,
the attributes end in a subdir, without a name, they will be created
directly.

Kay
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/