On Tue, Apr 28, 2015 at 07:28:21PM +0200, Stefan Agner wrote: > On 2015-04-28 18:31, Felipe Balbi wrote: > > On Tue, Apr 28, 2015 at 06:15:51PM +0200, Krzysztof Opasiak wrote: > >> On 04/28/2015 05:59 PM, Stefan Agner wrote: > >> >On 2015-04-28 17:00, Krzysztof Opasiak wrote: > >> >>Hi Stefan, > >> >> > >> >>On 04/28/2015 01:51 PM, Stefan Agner wrote: > >> >>>MAC addresses can be written without leading zeros. A popular > >> >>>example is libc's ether_ntoa_r function which creates such > >> >>>MAC addresses. > >> >>> > >> >>>Example: > >> >>>00:14:3d:0f:ff:fe can be written as 0:14:3d:f:ff:fe > >> >>> > >> >>>Additionally, get_ether_addr potentially parsed past the end > >> >>>of the user provided string. Use the opportunity and fix the > >> >>>function to never parse beyond the end of the string while > >> >>>allowing MAC addresses with and without leading zeros. > >> >>> > >> >>>Signed-off-by: Stefan Agner <stefan@xxxxxxxx> > >> > >> (...) > >> > >> >> > >> >>First of all thank you for your remark about reading past the buffer > >> >>in my version of this patch. I tried to make my change as little as > >> >>possible and didn't catch that this function is broken from beginning. > >> >> > >> >>This one is definitely better in this case but I'm afraid that it > >> >>won't be able to correctly interpret MAC address written without > >> >>separators while original version works fine for it. > >> > > >> >That is true, we need separators to be able to parse the address now. Do > >> >we need to support the format without separators? The sysfs > >> >documentation is not very clear about the supported format (e.g. > >> >Documentation/ABI/testing/configfs-usb-gadget-ecm). > >> > > >> >Having variable length of the individual bytes and no separators for the > >> >individual bytes somehow contradicts. We can of course just assume that > >> >we have 2 characters when using no separators, its just somewhat > >> >irregular... > >> > >> Let's check what is currently supported: > >> > >> 1) 00:14:3d:0f:ff:fe (no skipped 0, semicolons) > >> 2) 00.14.3d.0f.ff.fe (no skipped 0, dots) > >> 3) 00143d0ffffe (no skipped 0, no separator) > >> > >> What we are actually trying to do is adding a new format: > >> > >> 4) 0:14:3d:f:ff:fe (skipped leading 0, semicolons) > >> 5) 0.14.3d.f.ff.fe (skipped leading 0, dots) > >> > >> In my humble opinion adding a new format in this case is a very good idea > >> but it should not affect any other format that is currently supported by > >> this function > > > > agreed :-) > > I would have traded the simpler and more robust code with format 3 :-) > > However, when preserving format 3, my parsing approach is probably not > suited, since it requires the separator to increment the destination > arrays index. I will see what I can do. > > Btw, one other odd format which is used by module parameters is coma > separated bytes with hexadecimal indicators: > 0x00,0x04,0x9f,0x01,0x30,0xe0 > > Which allows to use the module_param_array. But probably definitely out > of scope here. yeah, that's probably never gonna be used though as we can't add per-instance module parameters. -- balbi
Attachment:
signature.asc
Description: Digital signature
