[PATCH] Replace strncpy with memccpy to fix -Wstringop-truncation.

[PATCH] Replace strncpy with memccpy to fix -Wstringop-truncation.

[Khem Raj]

	* nis/nss_nisplus/nisplus-parser.c: Replace strncpy with memcpy to
	avoid -Wstringop-truncation.
---
2018-03-22  Khem Raj  <raj.khem@gmail.com>

        * nis/nss_nisplus/nisplus-parser.c (_nss_nisplus_parse_pwent): Replace
        strncpy with memcpy to avoid -Wstringop-truncation.

 nis/nss_nisplus/nisplus-parser.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/nis/nss_nisplus/nisplus-parser.c b/nis/nss_nisplus/nisplus-parser.c
index 8dc021e73d..b53284f889 100644
--- a/nis/nss_nisplus/nisplus-parser.c
+++ b/nis/nss_nisplus/nisplus-parser.c
@@ -87,7 +87,7 @@ _nss_nisplus_parse_pwent (nis_result *result, struct passwd *pw,
       if (len >= room_left)
 	goto no_more_room;
 
-      strncpy (first_unused, numstr, len);
+      memcpy (first_unused, numstr, len);
       first_unused[len] = '\0';
       numstr = first_unused;
     }
@@ -103,7 +103,7 @@ _nss_nisplus_parse_pwent (nis_result *result, struct passwd *pw,
       if (len >= room_left)
 	goto no_more_room;
 
-      strncpy (first_unused, numstr, len);
+      memcpy (first_unused, numstr, len);
       first_unused[len] = '\0';
       numstr = first_unused;
     }
-- 
2.16.2

Re: [PATCH] Replace strncpy with memccpy to fix -Wstringop-truncation.

[Joseph Myers]

On Thu, 22 Mar 2018, Khem Raj wrote:

> 	* nis/nss_nisplus/nisplus-parser.c: Replace strncpy with memcpy to
> 	avoid -Wstringop-truncation.

Could you give more details of (a) in what circumstances (architecture, 
compiler, etc.) you get this warning and (b) why the truncation is 
correct?

Whatever build failure you got hasn't shown up with build-many-glibcs.py.  
Maybe that should include a configuration with --enable-obsolete-rpc 
--enable-obsolete-nsl to make sure that it tests building those bits of 
code that are disabled by default?

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [PATCH] Replace strncpy with memccpy to fix -Wstringop-truncation.

[Khem Raj]

Hi Joseph

On Thu, Mar 22, 2018 at 6:17 PM, Joseph Myers <joseph@codesourcery.com> wrote:
> On Thu, 22 Mar 2018, Khem Raj wrote:
>
>>       * nis/nss_nisplus/nisplus-parser.c: Replace strncpy with memcpy to
>>       avoid -Wstringop-truncation.
>
> Could you give more details of (a) in what circumstances (architecture,
> compiler, etc.) you get this warning and (b) why the truncation is
> correct?
>
> Whatever build failure you got hasn't shown up with build-many-glibcs.py.
> Maybe that should include a configuration with --enable-obsolete-rpc
> --enable-obsolete-nsl to make sure that it tests building those bits of
> code that are disabled by default?
>

This is seen with gcc/trunk when cross compiling for armv7ve target
and yes --enable-obsolete-rpc is used to configure, I am using
OpenEmbedded build system.

nss_nisplus/nisplus-parser.c:90:7: error: 'strncpy' destination
unchanged after copying no bytes [-Werror=stringop-truncation]
       strncpy (first_unused, numstr, len);
       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
nss_nisplus/nisplus-parser.c:106:7: error: 'strncpy' destination
unchanged after copying no bytes [-Werror=stringop-truncation]
       strncpy (first_unused, numstr, len);
       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Re: [PATCH] Replace strncpy with memccpy to fix -Wstringop-truncation.

[Stefan Liebler]

On 03/23/2018 02:29 AM, Khem Raj wrote:
> Hi Joseph
> 
> On Thu, Mar 22, 2018 at 6:17 PM, Joseph Myers <joseph@codesourcery.com> wrote:
>> On Thu, 22 Mar 2018, Khem Raj wrote:
>>
>>>        * nis/nss_nisplus/nisplus-parser.c: Replace strncpy with memcpy to
>>>        avoid -Wstringop-truncation.
>>
>> Could you give more details of (a) in what circumstances (architecture,
>> compiler, etc.) you get this warning and (b) why the truncation is
>> correct?
>>
>> Whatever build failure you got hasn't shown up with build-many-glibcs.py.
>> Maybe that should include a configuration with --enable-obsolete-rpc
>> --enable-obsolete-nsl to make sure that it tests building those bits of
>> code that are disabled by default?
>>
> 
> This is seen with gcc/trunk when cross compiling for armv7ve target
> and yes --enable-obsolete-rpc is used to configure, I am using
> OpenEmbedded build system.
> 
> nss_nisplus/nisplus-parser.c:90:7: error: 'strncpy' destination
> unchanged after copying no bytes [-Werror=stringop-truncation]
>         strncpy (first_unused, numstr, len);
>         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> nss_nisplus/nisplus-parser.c:106:7: error: 'strncpy' destination
> unchanged after copying no bytes [-Werror=stringop-truncation]
>         strncpy (first_unused, numstr, len);
>         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 
Hi,

I also see those warnings on s390x if build with gcc-head (from today) 
and if glibc is configured with --enable-obsolete-nsl.

Why do we need the strncpy at all?
if (len == 0 && ...)
{
	...
	strncpy (first_unused, numstr, len);
	first_unused[len] = '\0';
	...
}

Bye.
Stefan

Re: [PATCH] Replace strncpy with memccpy to fix -Wstringop-truncation.

[Andreas Schwab]

On Apr 05 2018, Stefan Liebler <stli@linux.vnet.ibm.com> wrote:

> Why do we need the strncpy at all?
> if (len == 0 && ...)

That's obviously a typo.

Andreas.

-- 
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

Re: [PATCH] Replace strncpy with memccpy to fix -Wstringop-truncation.

[Stefan Liebler]

[-- Attachment #1: Type: text/plain, Size: 683 bytes --]

On 04/05/2018 06:14 PM, Andreas Schwab wrote:
> On Apr 05 2018, Stefan Liebler <stli@linux.vnet.ibm.com> wrote:
> 
>> Why do we need the strncpy at all?
>> if (len == 0 && ...)
> 
> That's obviously a typo.
> 
> Andreas.
> 
Yes. You are right. Please have a look at the applied patch.
If the zero-termination is needed, numstr is copied to the buffer with 
strncpy and the zero-termination is applied.
If numstr is either 0 bytes long or the length of the numstr string is 
0, then _nss_nisplus_parse_pwent returns with 0.


This solves the mentioned warning with if build with gcc-head and 
--enable-obsolete-nsl.
But I can not test it as I don't have a nisplus setup.

Bye.
Stefan

[-- Attachment #2: 20180412_nisplus-parser.patch --]
[-- Type: text/x-patch, Size: 1388 bytes --]

diff --git a/nis/nss_nisplus/nisplus-parser.c b/nis/nss_nisplus/nisplus-parser.c
index 8dc021e73d..4714a3085a 100644
--- a/nis/nss_nisplus/nisplus-parser.c
+++ b/nis/nss_nisplus/nisplus-parser.c
@@ -82,7 +82,7 @@ _nss_nisplus_parse_pwent (nis_result *result, struct passwd *pw,
 
   char *numstr = NISOBJVAL (2, obj);
   len = NISOBJLEN (2, obj);
-  if (len == 0 && numstr[len - 1] != '\0')
+  if (len != 0 && numstr[len - 1] != '\0')
     {
       if (len >= room_left)
 	goto no_more_room;
@@ -91,14 +91,14 @@ _nss_nisplus_parse_pwent (nis_result *result, struct passwd *pw,
       first_unused[len] = '\0';
       numstr = first_unused;
     }
-  if (numstr[0] == '\0')
+  if (len == 0 || numstr[0] == '\0')
     /* If we don't have a uid, it's an invalid shadow entry.  */
     return 0;
   pw->pw_uid = strtoul (numstr, NULL, 10);
 
   numstr = NISOBJVAL (3, obj);
   len = NISOBJLEN (3, obj);
-  if (len == 0 && numstr[len - 1] != '\0')
+  if (len != 0 && numstr[len - 1] != '\0')
     {
       if (len >= room_left)
 	goto no_more_room;
@@ -107,7 +107,7 @@ _nss_nisplus_parse_pwent (nis_result *result, struct passwd *pw,
       first_unused[len] = '\0';
       numstr = first_unused;
     }
-  if (numstr[0] == '\0')
+  if (len == 0 || numstr[0] == '\0')
     /* If we don't have a gid, it's an invalid shadow entry.  */
     return 0;
   pw->pw_gid = strtoul (numstr, NULL, 10);

Re: [PATCH] Replace strncpy with memccpy to fix -Wstringop-truncation.

[Stefan Liebler]

On 04/12/2018 05:31 PM, Stefan Liebler wrote:
> On 04/05/2018 06:14 PM, Andreas Schwab wrote:
>> On Apr 05 2018, Stefan Liebler <stli@linux.vnet.ibm.com> wrote:
>>
>>> Why do we need the strncpy at all?
>>> if (len == 0 && ...)
>>
>> That's obviously a typo.
>>
>> Andreas.
>>
> Yes. You are right. Please have a look at the applied patch.
> If the zero-termination is needed, numstr is copied to the buffer with 
> strncpy and the zero-termination is applied.
> If numstr is either 0 bytes long or the length of the numstr string is 
> 0, then _nss_nisplus_parse_pwent returns with 0.
> 
> 
> This solves the mentioned warning with if build with gcc-head and 
> --enable-obsolete-nsl.
> But I can not test it as I don't have a nisplus setup.
> 
> Bye.
> Stefan

PING