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;
