netdev - Re: [PATCH net-next 6/6] amd-xgbe: Resolve checkpatch warning about sscanf usage

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Thu, 26 Jun 2014 17:12:55 -0700 (PDT)
From:	David Miller <davem@...emloft.net>
To:	joe@...ches.com
Cc:	thomas.lendacky@....com, netdev@...r.kernel.org
Subject: Re: [PATCH net-next 6/6] amd-xgbe: Resolve checkpatch warning
 about sscanf usage

From: Joe Perches <joe@...ches.com>
Date: Tue, 24 Jun 2014 15:53:30 -0700

> On Tue, 2014-06-24 at 17:44 -0500, Tom Lendacky wrote:
>> On 06/24/2014 05:00 PM, Joe Perches wrote:
>> > On Tue, 2014-06-24 at 16:19 -0500, Tom Lendacky wrote:
>> >> Checkpatch issued a warning preferring to use kstrto<type> when
>> >> using a single variable sscanf.  Change the sscanf invocation to
>> >> a kstrtouint call.
>> > []
>> >> diff --git a/drivers/net/ethernet/amd/xgbe/xgbe-debugfs.c b/drivers/net/ethernet/amd/xgbe/xgbe-debugfs.c
>> > []
>> >> @@ -165,10 +165,9 @@ static ssize_t xgbe_common_write(const char __user *buffer, size_t count,
>> >>   		return len;
>> >>
>> >>   	workarea[len] = '\0';
>> >> -	if (sscanf(workarea, "%x", &scan_value) == 1)
>> >> -		*value = scan_value;
>> >> -	else
>> >> -		return -EIO;
>> >> +	ret = kstrtouint(workarea, 0, value);
>> >
>> > Don't you need to use 16 for the base here?
> 
>> Using 0 allows for greater flexibility in the input format.
> 
> True, but there could be a change in behavior like reading a
> previously hex value like 10 is now a decimal 10 not decimal 16.

Tom, under other circumstance you can't change the format.
v3.16 is going to be released with the existing %x formatting
expecting hexadecimal numbers.

And you're targetting this change to decimal format in net-next.

The only thing that really allows you to do this is that this is
debugfs, and it's a reason I really hate debugfs, people do
arbitrary stuff so that if the debugfs elements turn out to be
useful for someone the driver author can arbitarily break things
on them however they want.

It's a cop-out for things people don't want to be bound to avoid ABI
changes, and to me that's garbage.  If you expose it to the user
design it well to the point where you're willing to live with it's
interface forever, or don't expose it to the user at all.
--
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