> On Sep 14, 2021, at 12:48 PM, Chuck Lever III <chuck.lever@xxxxxxxxxx> wrote:
>
>
>
>> On Sep 14, 2021, at 12:37 PM, Bruce Fields <bfields@xxxxxxxxxxxx> wrote:
>>
>> From: "J. Bruce Fields" <bfields@xxxxxxxxxx>
>> Subject: [PATCH] nfsd: don't alloc under spinlock in rpc_parse_scope_id
>>
>> Dan Carpenter says:
>>
>> The patch d20c11d86d8f: "nfsd: Protect session creation and client
>> confirm using client_lock" from Jul 30, 2014, leads to the following
>> Smatch static checker warning:
>>
>> net/sunrpc/addr.c:178 rpc_parse_scope_id()
>> warn: sleeping in atomic context
>>
>> Reported-by: Dan Carpenter <dan.carpenter@xxxxxxxxxx>
>> Fixes: d20c11d86d8f ("nfsd: Protect session creation and client...")
>> Signed-off-by: J. Bruce Fields <bfields@xxxxxxxxxx>
>> ---
>>
>> net/sunrpc/addr.c | 40 ++++++++++++++++++----------------------
>> 1 file changed, 18 insertions(+), 22 deletions(-)
>>
>> On Thu, Sep 09, 2021 at 10:56:33AM -0400, Jeff Layton wrote:
>>> Hmm, it sounds line in the second email he suggests using memcpy():
>>>
>>> "Your "memcpy()" example implies that the source is always a fixed-size
>>> thing. In that case, maybe that's the rigth thing to do, and you
>>> should just create a real function for it."
>>>
>>> Maybe I'm missing the context though.
>
> The scope identifier isn't fixed in size, so I'm not sure how you
> got there.
>
>
>>> In any case, when you're certain about the length of the source and
>>> destination buffers, there's no real benefit to avoiding memcpy in favor
>>> of strcpy and the like. It's just as correct.
>>
>> OK, queueing this up as is for 5.16 unless someone objects.
>
> IMO Linus prefers strscpy() over open-coded memcpys, but it's not
> a hill I'm going to fight and die on.
>
>
>> (But, could
>> really use testing, I'm not currently testing over ipv6.)--b.
>
> Seems like you could generate some artificial test cases without
> needing to set up IPv6.
Actually, a scope ID is needed when using link-local addresses,
which are set up automatically simply when IPv6 is enabled.
If you see an address like this:
inet6 fe80::2eae:686b:8c82:fad2/64 scope link noprefixroute
valid_lft forever preferred_lft forever
Then you can use this address to mount with by adding that
interface name as the scope ID.
>> diff --git a/net/sunrpc/addr.c b/net/sunrpc/addr.c
>> index 6e4dbd577a39..d435bffc6199 100644
>> --- a/net/sunrpc/addr.c
>> +++ b/net/sunrpc/addr.c
>> @@ -162,8 +162,10 @@ static int rpc_parse_scope_id(struct net *net, const char *buf,
>> const size_t buflen, const char *delim,
>> struct sockaddr_in6 *sin6)
>> {
>> - char *p;
>> + char p[IPV6_SCOPE_ID_LEN + 1];
>> size_t len;
>> + u32 scope_id = 0;
>> + struct net_device *dev;
>>
>> if ((buf + buflen) == delim)
>> return 1;
>> @@ -175,29 +177,23 @@ static int rpc_parse_scope_id(struct net *net, const char *buf,
>> return 0;
>>
>> len = (buf + buflen) - delim - 1;
>> - p = kmemdup_nul(delim + 1, len, GFP_KERNEL);
>> - if (p) {
>> - u32 scope_id = 0;
>> - struct net_device *dev;
>> -
>> - dev = dev_get_by_name(net, p);
>> - if (dev != NULL) {
>> - scope_id = dev->ifindex;
>> - dev_put(dev);
>> - } else {
>> - if (kstrtou32(p, 10, &scope_id) != 0) {
>> - kfree(p);
>> - return 0;
>> - }
>> - }
>> -
>> - kfree(p);
>> -
>> - sin6->sin6_scope_id = scope_id;
>> - return 1;
>> + if (len > IPV6_SCOPE_ID_LEN)
>> + return 0;
>> +
>> + memcpy(p, delim + 1, len);
>> + p[len] = 0;
>> +
>> + dev = dev_get_by_name(net, p);
>> + if (dev != NULL) {
>> + scope_id = dev->ifindex;
>> + dev_put(dev);
>> + } else {
>> + if (kstrtou32(p, 10, &scope_id) != 0)
>> + return 0;
>> }
>>
>> - return 0;
>> + sin6->sin6_scope_id = scope_id;
>> + return 1;
>> }
>>
>> static size_t rpc_pton6(struct net *net, const char *buf, const size_t buflen,
>> --
>> 2.31.1
>>
>
> --
> Chuck Lever
--
Chuck Lever
