linux-kernel - Re: [PATCH 2/2] SUNRPC: Optimize 'svc_print

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: Windows password security audit tool. GUI, reports in PDF.

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

Date:   Thu, 26 Mar 2020 18:29:41 -0400
From:   Chuck Lever <chuck.lever@...cle.com>
To:     Neil Brown <neilb@...e.de>,
        Christophe JAILLET <christophe.jaillet@...adoo.fr>
Cc:     Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Linux NFS Mailing List <linux-nfs@...r.kernel.org>,
        kernel-janitors@...r.kernel.org
Subject: Re: [PATCH 2/2] SUNRPC: Optimize 'svc_print_xprts()'



> On Mar 26, 2020, at 5:44 PM, NeilBrown <neilb@...e.de> wrote:
> 
> On Thu, Mar 26 2020, Christophe JAILLET wrote:
> 
>> Le 25/03/2020 à 23:53, NeilBrown a écrit :
>>> Can I suggest something more like this:
>>> diff --git a/net/sunrpc/svc_xprt.c b/net/sunrpc/svc_xprt.c
>>> index de3c077733a7..0292f45b70f6 100644
>>> --- a/net/sunrpc/svc_xprt.c
>>> +++ b/net/sunrpc/svc_xprt.c
>>> @@ -115,16 +115,9 @@ int svc_print_xprts(char *buf, int maxlen)
>>>  	buf[0] = '\0';
>>> 
>>>  	spin_lock(&svc_xprt_class_lock);
>>> -	list_for_each_entry(xcl, &svc_xprt_class_list, xcl_list) {
>>> -		int slen;
>>> -
>>> -		sprintf(tmpstr, "%s %d\n", xcl->xcl_name, xcl->xcl_max_payload);
>>> -		slen = strlen(tmpstr);
>>> -		if (len + slen > maxlen)
>>> -			break;
>>> -		len += slen;
>>> -		strcat(buf, tmpstr);
>>> -	}
>>> +	list_for_each_entry(xcl, &svc_xprt_class_list, xcl_list)
>>> +		len += scnprintf(buf + len, maxlen - len, "%s %d\n",
>>> +				 xcl->xcl_name, xcl->xcl_max_payload);
>>>  	spin_unlock(&svc_xprt_class_lock);
>>> 
>>>  	return len;
>>> 
>>> NeilBrown
>> 
>> Hi,
>> 
>> this was what I suggested in the patch:
>>     ---
>>     This patch should have no functional change.
>>     We could go further, use scnprintf and write directly in the 
>> destination
>>     buffer. However, this could lead to a truncated last line.
>>     ---
> 
> Sorry - I missed that.
> So add
> 
> end = strrchr(tmpstr, '\n');
> if (end)
>    end[1] = 0;
> else
>    tmpstr[0] = 0;
> 
> or maybe something like
> 	list_for_each_entry(xcl, &svc_xprt_class_list, xcl_list) {
> 		int l = snprintf(buf + len, maxlen - len, "%s %d\n",
> 				 xcl->xcl_name, xcl->xcl_max_payload);
>                if (l < maxlen - len)
>                	len += l;
>        }
>        buf[len] = 0;
> 
> There really is no need to have the secondary buffer, and I think doing
> so just complicates the code.

In the interest of getting this fix into the upcoming merge window, let's
stick with the temporary buffer approach. Thanks!


--
Chuck Lever