javacjava-native-interfacecan-bussocketcan

Why does SocketCAN bind() call always return 0 when called from JNI?


I'm attempting to create a Java wrapped C implementation of a CAN bus that is to be used in an Android environment. I've used SocketCAN including in the Linux kernel to create a socket and bind it to the CAN interface.

The development environment doesn't have a physical CAN bus, and as a result I'm creating a virtual bus via sudo ip link add dev vcan0 type vcan and sudo ip link set up vcan0.

Running the native C code in this environment works as expected, the socket binds when the interface is present, and returns an error when it is not. However, when running the same native C code via JNI the bind(...) call always returns 0 regardless of the state of the interface, although any subsequent write(...) calls fail as expected.

Is there something I've overlooked that means this is the case?

The JNI code is as follows (lifted directly from my C implementation with additional type casting where necessary):

JNIEXPORT jboolean JNICALL Java_SocketCAN_nativeOpen
  (JNIEnv * env, jobject jobj, jint bus_id)
{
  if ((int) bus_id < MAX_NUMBER_OF_CAN_BUSES)
  {
    int s;

    if((s = socket(PF_CAN, SOCK_RAW, CAN_RAW)) == -1) 
    {
      printf("Error while opening socket\n");
      return JNI_FALSE;
    }

    struct sockaddr_can addr;
    struct ifreq ifr;

    strcpy(ifr.ifr_name, "vcan0");
    ioctl(s, SIOCGIFINDEX, &ifr);
    
    addr.can_family  = AF_CAN;
    addr.can_ifindex = ifr.ifr_ifindex;

    if(bind(s, (struct sockaddr *)&addr, sizeof(addr)) == -1) 
    {
      printf("Error in socket bind\n");
      return JNI_FALSE;
    }

    // Set the socketId in the Java class.
    jclass   jcls        = (*env)->FindClass(env, "SocketCAN");
    jfieldID socket_id   = (*env)->GetFieldID(env, jcls, "socket", "I");
    jint     j_socket_id = (*env)->GetIntField(env, jobj, socket_id);
    j_socket_id = s;
    (*env)->SetIntField(env, jobj, socket_id, j_socket_id);

    return JNI_TRUE;
  }
  
  return JNI_FALSE;
}

Any help is much appreciated, thanks!

EDIT: If anyone seems to be experiencing this weird issue and wants a workaround (although it might be the correct way to do this and I've overlooked it), check the return value from the ioctl(...) function call. That returns -1 when "vcan0" isn't set up when running both the C and the JNI.

My updated code after modifying for the suggestions made by @12431234123412341234123 and @AndrewHenle is as follows:

JNIEXPORT jint JNICALL Java_SocketCAN_nativeOpen
  (JNIEnv * env, jobject jobj, jint bus_id)
{
  if ((int) bus_id < MAX_NUMBER_OF_CAN_BUSES)
  {
    int s;

    if((s = socket(PF_CAN, SOCK_RAW, CAN_RAW)) == -1) 
    {
      printf("Error while opening socket\n");
      return -1;
    }

    struct sockaddr_can addr;
    struct ifreq ifr;

    strcpy(ifr.ifr_name, "vcan0");
    if (ioctl(s, SIOCGIFINDEX, &ifr) == -1)
    {
      printf("Error in ioctl\n");
      return -1;
    }
    
    addr.can_family  = AF_CAN;
    addr.can_ifindex = ifr.ifr_ifindex;

    if(bind(s, (struct sockaddr *)&addr, sizeof(addr)) == -1) 
    {
      printf("Error in socket bind\n");
      return -1;
    }

    return (jint) s;
  }
  
  return -1;
}

Solution

  • The bind() call needs a proper interface index in can_ifindex. To get this value you can use the ioctl() call with SIOCGIFINDEX, as you do. However, when the ioctl() call fails, the ifreq structure does not necessarily have the correct index, it probably still has the "random" value from the last object that occupied the same memory region before. Because you ignored the return value from ioctl(), you called bind() with a "random" interface index. This also means that bind may or may not fail, depending on the value, because using a uninitialized value is UB in most cases. To avoid this error, check for the return value from ioctl() and handle errors accordingly.

    It seems that this "random" value is different for the plain C version as it is for the JNI version. A possibility to avoid such random differences is by setting every new automatic object directly to a value. In your case you could to set everything to 0: struct ifreq ifr={0};, same for addr. This extra step could gain a more consistent behaviour.