Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1646261

Re: [PATCH 10/24] thunderbolt: Read vendor and device name from DROM

From Lukas Wunner <lukas@wunner.de>
Newsgroups linux.kernel
Subject Re: [PATCH 10/24] thunderbolt: Read vendor and device name from DROM
Date 2017-05-21 07:40 +0200
Message-ID <tJrKV-3Ec-7@gated-at.bofh.it> (permalink)
References <tIuKR-3YK-5@gated-at.bofh.it> <tIuKS-3YK-15@gated-at.bofh.it> <tIN18-GC-25@gated-at.bofh.it> <tINkt-Ng-3@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


On Fri, May 19, 2017 at 01:28:36PM +0300, Mika Westerberg wrote:
> On Fri, May 19, 2017 at 12:07:10PM +0200, Lukas Wunner wrote:
> > Apple uses 0x30 to store a
> > serial number.  Is this attribute number assigned by Intel to Apple
> > or is it reserved for vendor use or did they arbitrarily choose it?
> 
> It is part of the DROM specification. The 0x30 - 0x3e are vendor
> specific entries.

Ah, so I have to qualify the vendor number with Apple's ID before I know
that it's a serial number.  Thanks.


> > If there can be many attributes, should they be stored in a list
> > rather than adding a char* pointer for each one to struct tb_switch?
> > The latter doesn't scale.
> 
> I don't think we need other attributes (well, at least right now). The
> device/vendor name is useful because that's what we expose to the
> userspace for device identification along with the device/vendor ID.

Okay.  It might be worth to log additional attributes with info level.


> > > +static void tb_drom_parse_generic_entry(struct tb_switch *sw,
> > > +		struct tb_drom_entry_generic *entry)
> > > +{
> > > +	if (entry->header.index == 1)
> > > +		sw->vendor_name = kstrdup((char *)entry->data, GFP_KERNEL);
> > > +	else if (entry->header.index == 2)
> > > +		sw->device_name = kstrdup((char *)entry->data, GFP_KERNEL);
> > > +}
> > 
> > This assumes that these are properly null-terminated strings, but the DROM
> > may contain complete garbage.  The existing drom parser is very careful
> > to validate and sanitize everything.
> 
> The DROM specification says they must be null-terminated but I yes, it
> is possible that some of the devices have it wrong. The generic entry
> includes length field so I suppose we can use that + kmemdup() instead
> here?

Yes, as long as you check that the last character is null.

Thanks,

Lukas

Back to linux.kernel | Previous | NextPrevious in thread | Next in thread | Find similar | Unroll thread


Thread

[PATCH 10/24] thunderbolt: Read vendor and device name from DROM Mika Westerberg <mika.westerberg@linux.intel.com> - 2017-05-18 16:40 +0200
  Re: [PATCH 10/24] thunderbolt: Read vendor and device name from DROM Andy Shevchenko <andy.shevchenko@gmail.com> - 2017-05-18 21:20 +0200
    Re: [PATCH 10/24] thunderbolt: Read vendor and device name from DROM Mika Westerberg <mika.westerberg@linux.intel.com> - 2017-05-19 10:30 +0200
  Re: [PATCH 10/24] thunderbolt: Read vendor and device name from DROM Lukas Wunner <lukas@wunner.de> - 2017-05-19 12:10 +0200
    Re: [PATCH 10/24] thunderbolt: Read vendor and device name from DROM Mika Westerberg <mika.westerberg@linux.intel.com> - 2017-05-19 12:30 +0200
      Re: [PATCH 10/24] thunderbolt: Read vendor and device name from DROM Lukas Wunner <lukas@wunner.de> - 2017-05-21 07:40 +0200
        Re: [PATCH 10/24] thunderbolt: Read vendor and device name from DROM Mika Westerberg <mika.westerberg@linux.intel.com> - 2017-05-21 09:50 +0200
          Re: [PATCH 10/24] thunderbolt: Read vendor and device name from DROM Lukas Wunner <lukas@wunner.de> - 2017-05-21 11:40 +0200

csiph-web