Build error with GCC 10 in NetworkInterface.c and k_standard.c
Koichi Sakata
sakatakui at oss.nttdata.com
Mon Jul 13 07:03:07 UTC 2020
Hi Daniel,
On 2020/07/09 18:16, Daniel Fuchs wrote:
> 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.
I understand that. I respect your idea.
I fixed the patch as follows.
By the way, k_standard.c still remains. Is there a way to proceed with it?
Thanks,
Koichi
===== PATCH =====
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
@@ -1296,7 +1296,8 @@
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);
+ strncpy(if2.ifr_name, name, sizeof(if2.ifr_name));
+ if2.ifr_name[sizeof(if2.ifr_name) - 1] = 0;
if (ioctl(sock, SIOCGIFINDEX, (char *)&if2) < 0) {
return -1;
@@ -1359,7 +1360,8 @@
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);
+ strncpy(if2.ifr_name, ifname, sizeof(if2.ifr_name));
+ if2.ifr_name[sizeof(if2.ifr_name) - 1] = 0;
if (ioctl(sock, SIOCGIFFLAGS, (char *)&if2) < 0) {
return -1;
diff --git a/src/java.base/share/native/libfdlibm/k_standard.c
b/src/java.base/share/native/libfdlibm/k_standard.c
--- a/src/java.base/share/native/libfdlibm/k_standard.c
+++ b/src/java.base/share/native/libfdlibm/k_standard.c
@@ -739,6 +739,10 @@
errno = EDOM;
}
break;
+ default:
+ exc.retval = zero;
+ errno = EINVAL;
+ break;
}
return exc.retval;
}
>
> 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