Build error with GCC 10 in NetworkInterface.c and k_standard.c
Daniel Fuchs
daniel.fuchs at oracle.com
Thu Jul 9 09:16:46 UTC 2020
Hi Koichi,
If you don't mind I'd prefer to keep the simple approach
that will fix the warning without changing the behavior
of the function.
memset((char *)&if2, 0, sizeof(if2));
- strncpy(if2.ifr_name, name, sizeof(if2.ifr_name) - 1);
+ strncpy(if2.ifr_name, name, sizeof(if2.ifr_name));
+ if2.ifr_name[sizeof(if2.ifr_name) - 1] = 0;
or at the very least use `strnlen` instead of `strlen`:
if (sizeof(if2.ifr_name) < strnlen(name, sizeof(if2.ifr_name)) + 1) {
return -1;
}
that would be acceptable too.
best regards,
-- daniel
On 09/07/2020 08:48, Koichi Sakata wrote:
> Hi Daniel,
>
> Thank you for your response.
>
> > > + if (sizeof(if2.ifr_name) < sizeof(name)) {
> > > + return -1;
> > > + }
> >
> > If I'm not mistaken `sizeof(name)` where name is a const char*
> > will always be 8 (on 64 bits architecture) - so this is probably
> > not doing what you want.
>
> I'm sorry. That is my mistake.
>
> > Also you don't want to trust that `name` is null terminated.
> > Calling strncpy in that case is much safer than calling strcpy
> > even if guarded by strlen.
> > I'm not sure what the compiler is warning you about here.
> >
> > I'd suggest to experiment changing:
> >
> > > memset((char *)&if2, 0, sizeof(if2));
> > > - strncpy(if2.ifr_name, name, sizeof(if2.ifr_name) - 1);
> > + strncpy(if2.ifr_name, name, sizeof(if2.ifr_name));
> > + if2.ifr_name[sizeof(if2.ifr_name) - 1] = 0;
>
> I've written the code same as your suggestion. Certainly that can
> resolve the warnings. But I think it could cut off the name in the
> middle when length of the name longer than the if2.ifr_name length. I
> would say I might as well return the error code under the circumstances.
> So I added the if statement. The code that reflect my intention
> correctly is as follows.
>
>
> diff --git a/src/java.base/unix/native/libnet/NetworkInterface.c
> b/src/java.base/unix/native/libnet/NetworkInterface.c
> --- a/src/java.base/unix/native/libnet/NetworkInterface.c
> +++ b/src/java.base/unix/native/libnet/NetworkInterface.c
> @@ -1295,8 +1295,13 @@
> */
> static int getIndex(int sock, const char *name) {
> struct ifreq if2;
> +
> + if (sizeof(if2.ifr_name) < strlen(name) + 1) {
> + return -1;
> + }
> +
> memset((char *)&if2, 0, sizeof(if2));
> - strncpy(if2.ifr_name, name, sizeof(if2.ifr_name) - 1);
> + strcpy(if2.ifr_name, name);
>
> if (ioctl(sock, SIOCGIFINDEX, (char *)&if2) < 0) {
> return -1;
> @@ -1358,8 +1363,13 @@
>
> static int getFlags(int sock, const char *ifname, int *flags) {
> struct ifreq if2;
> +
> + if (sizeof(if2.ifr_name) < strlen(ifname) + 1) {
> + return -1;
> + }
> +
> memset((char *)&if2, 0, sizeof(if2));
> - strncpy(if2.ifr_name, ifname, sizeof(if2.ifr_name) - 1);
> + strcpy(if2.ifr_name, ifname);
>
> if (ioctl(sock, SIOCGIFFLAGS, (char *)&if2) < 0) {
> return -1;
>
>
> Thanks,
> Koichi
>
> On 2020/07/08 2:57, Daniel Fuchs wrote:
>> Hi,
>>
>> I will not comment on the changes to libfdlibm/k_standard.c
>>
>> Concerning NetworkInterface.c I believe the proposed changes are
>> incorrect - and I do not see the issue with the current code.
>>
>> > + if (sizeof(if2.ifr_name) < sizeof(name)) {
>> > + return -1;
>> > + }
>>
>> If I'm not mistaken `sizeof(name)` where name is a const char*
>> will always be 8 (on 64 bits architecture) - so this is probably
>> not doing what you want.
>>
>> Also you don't want to trust that `name` is null terminated.
>> Calling strncpy in that case is much safer than calling strcpy
>> even if guarded by strlen.
>> I'm not sure what the compiler is warning you about here.
>>
>> I'd suggest to experiment changing:
>>
>> > memset((char *)&if2, 0, sizeof(if2));
>> > - strncpy(if2.ifr_name, name, sizeof(if2.ifr_name) - 1);
>> + strncpy(if2.ifr_name, name, sizeof(if2.ifr_name));
>> + if2.ifr_name[sizeof(if2.ifr_name) - 1] = 0;
>>
>> and see if that makes the warning go away.
>>
>> best regards,
>>
>> -- daniel
>>
>>
>> On 24/06/2020 08:48, Koichi Sakata wrote:
>>> Hi all,
>>>
>>> (I've sent a similar e-mail before to this ML, but I extract the part
>>> only related to core-libs-dev ML from the previous one.)
>>>
>>> I tried to build OpenJDK fastdebug with GCC 10.1 on Ubuntu 18.04, but
>>> I saw some compiler warnings as follows:
>>>
>>> /home/jyukutyo/code/jdk/src/java.base/share/native/libfdlibm/: In
>>> function '__j__kernel_standard':
>>> /home/jyukutyo/code/jdk/src/java.base/share/native/libfdlibm/k_standard.c:743:19:
>>> error: 'exc.retval' may be used uninitialized in this function
>>> [-Werror=maybe-uninitialized]
>>> 743 | return exc.retval;
>>> | ~~~^~~~~~~
>>>
>>> In file included from /usr/include/string.h:494,
>>> from
>>> /home/jyukutyo/code/jdk/src/java.base/unix/native/libnet/NetworkInterface.c:30:
>>>
>>> In function 'strncpy',
>>> inlined from 'getFlags' at
>>> /home/jyukutyo/code/jdk/src/java.base/unix/native/libnet/NetworkInterface.c:1362:5,
>>>
>>> inlined from 'addif' at
>>> /home/jyukutyo/code/jdk/src/java.base/unix/native/libnet/NetworkInterface.c:974:13:
>>>
>>> /usr/include/x86_64-linux-gnu/bits/string_fortified.h:106:10: error:
>>> '__builtin_strncpy' output may be truncated copying 15 bytes from a
>>> string of length 15 [-Werror=stringop-truncation]
>>> 106 | return __builtin___strncpy_chk (__dest, __src, __len,
>>> __bos (__dest));
>>> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>>
>>> I can resolve them with the following patch. I believe it fixes those
>>> potential bugs, so I'd like to contribute it.
>>> (Our company has signed OCA.)
>>>
>>> Thanks,
>>> Koichi
>>>
>>> ===== PATCH =====
>>> diff -r 20d92fe3ac52 src/java.base/share/native/libfdlibm/k_standard.c
>>> --- a/src/java.base/share/native/libfdlibm/k_standard.c Tue Jun 16
>>> 03:16:41 2020 +0000
>>> +++ b/src/java.base/share/native/libfdlibm/k_standard.c Thu Jun 18
>>> 07:08:50 2020 +0900
>>> @@ -739,6 +739,10 @@
>>> errno = EDOM;
>>> }
>>> break;
>>> + default:
>>> + exc.retval = zero;
>>> + errno = EINVAL;
>>> + break;
>>> }
>>> return exc.retval;
>>> }
>>> diff -r 20d92fe3ac52 src/java.base/unix/native/libnet/NetworkInterface.c
>>> --- a/src/java.base/unix/native/libnet/NetworkInterface.c Tue
>>> Jun 16 03:16:41 2020 +0000
>>> +++ b/src/java.base/unix/native/libnet/NetworkInterface.c Thu
>>> Jun 18 07:08:50 2020 +0900
>>> @@ -1296,7 +1296,10 @@
>>> static int getIndex(int sock, const char *name) {
>>> struct ifreq if2;
>>> memset((char *)&if2, 0, sizeof(if2));
>>> - strncpy(if2.ifr_name, name, sizeof(if2.ifr_name) - 1);
>>> + if (sizeof(if2.ifr_name) < sizeof(name)) {
>>> + return -1;
>>> + }
>>> + strcpy(if2.ifr_name, name);
>>>
>>> if (ioctl(sock, SIOCGIFINDEX, (char *)&if2) < 0) {
>>> return -1;
>>> @@ -1359,7 +1362,10 @@
>>> static int getFlags(int sock, const char *ifname, int *flags) {
>>> struct ifreq if2;
>>> memset((char *)&if2, 0, sizeof(if2));
>>> - strncpy(if2.ifr_name, ifname, sizeof(if2.ifr_name) - 1);
>>> + if (sizeof(if2.ifr_name) < sizeof(ifname)) {
>>> + return -1;
>>> + }
>>> + strcpy(if2.ifr_name, ifname);
>>>
>>> if (ioctl(sock, SIOCGIFFLAGS, (char *)&if2) < 0) {
>>> return -1;
>>
More information about the core-libs-dev
mailing list