On 2015-05-04 10:42, Krzysztof Opasiak wrote:
> Hi,
>
> On 04/28/2015 07:57 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
>>
>> The function get_ether_addr potentially also 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. Also
>> corner cases such as 00:14:3d:0f:ff:0 + new-line character
>> are parsed correctly.
>>
>> Signed-off-by: Stefan Agner <stefan@xxxxxxxx>
>> ---
>> drivers/usb/gadget/function/u_ether.c | 65 +++++++++++++++++++++++++++--------
>> 1 file changed, 51 insertions(+), 14 deletions(-)
>>
>> diff --git a/drivers/usb/gadget/function/u_ether.c b/drivers/usb/gadget/function/u_ether.c
>> index f1fd777..287a7dc 100644
>> --- a/drivers/usb/gadget/function/u_ether.c
>> +++ b/drivers/usb/gadget/function/u_ether.c
>> @@ -703,25 +703,57 @@ static int eth_stop(struct net_device *net)
>>
>> /*-------------------------------------------------------------------------*/
>>
>> +/**
>> + * get_ether_addr - parse ethernet address from string
>> + * @str: string to parse
>> + * @ethaddr: a buffer in which the parsed ethernet address will be
>> + * written
>
> param name is dev_addr not ethaddr
>
>> + *
>> + * Several common formats are 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)
>> + * 4) 0:14:3d:f:ff:fe (skipped leading 0, semicolons)
>> + * 5) 0.14.3d.f.ff.fe (skipped leading 0, dots)
>> + *
>> + * The function will not cross the end of the provided string even
>> + * when the string has the wrong format.
>> + *
>> + * Returns 0 on success, or -EINVAL on error
>> + */
>> static int get_ether_addr(const char *str, u8 *dev_addr)
>> {
>> - if (str) {
>> - unsigned i;
>> + int num, i;
>>
>> - for (i = 0; i < 6; i++) {
>> - unsigned char num;
>> + for (i = 0; i < ETH_ALEN; i++) {
>> + int j = 0;
>>
>> - if ((*str == '.') || (*str == ':'))
>> + dev_addr[i] = 0;
>> + while (*str) {
>> + char c = *str;
>> +
>> + if (c == '.' || c == ':') {
>> str++;
>> - num = hex_to_bin(*str++) << 4;
>> - num |= hex_to_bin(*str++);
>> - dev_addr [i] = num;
>> + break;
>> + }
>> +
>> + if (j >= 2)
>> + break;
>> +
>> + num = hex_to_bin(c);
>> + if (!(num < 0)) {
>> + dev_addr[i] <<= 4;
>> + dev_addr[i] |= num;
>> + }
>
> I'm not sure if it's a good idea. If you take only valid characters
> form string and skip all other you may parse "address" like:
>
> 00:Zf:3N:0M:fP:fG
>
> as
>
> 00:0f:03:00:0f:0f
>
> which may be a valid MAC address but in my opinion is totally not the
> thing that user expects.
>
> I would suggest to return error if invalid character has been placed
> in string instead of skipping.
When returning here with an error on a non-digit character, a normal
echo would fail due to the newline at then end. However, one could argue
that an ethernet address with a newline at the end is actually an
invalid address... With the correct options, one can also suppress the
newline when using echo:
echo -n 00:0f:03:00:0f:0f > dev_addr
However, since it worked before with the newline character, I wanted to
keep that for compatibility reasons.
We could of course add another exception for newline character... The
question which then need to be asked is the newline character the only
character we have to deal separately or could there be other character
we would like to ignore? I don't see other currently...
--
Stefan
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
