Skip to content

open a non-existent interface should always return 'no such interface' #1560

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

open a non-existent interface should always return 'no such interface' #1560

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
68 changes: 62 additions & 6 deletions pcap-linux.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -345,6 +345,7 @@ static const struct timeval netdown_timeout = {
static int iface_get_id(int fd, const char *device, char *ebuf);
static int iface_get_mtu(int fd, const char *device, char *ebuf);
static int iface_get_arptype(int fd, const char *device, char *ebuf);
static int iface_exists(const char *device, char *ebuf);
static int iface_bind(int fd, int ifindex, char *ebuf, int protocol);
static int enter_rfmon_mode(pcap_t *handle, int sock_fd,
const char *device);
Expand Down Expand Up @@ -2426,6 +2427,17 @@ setup_socket(pcap_t *handle, int is_any_device)
int err = 0;
struct packet_mreq mr;

/*
* Check if the interface exists before attempting to open
* the privileged packet socket, so we can return the correct
* error for non-existent interfaces.
*/
if (!is_any_device) {
status = iface_exists(device, handle->errbuf);
if (status != 0)
return status;
}

/*
* Open a socket with protocol family packet. If cooked is true,
* we open a SOCK_DGRAM socket for the cooked interface, otherwise
Expand Down Expand Up @@ -4812,6 +4824,50 @@ pcap_setfilter_linux(pcap_t *handle, struct bpf_program *filter)
return 0;
}

/*
* Check if the given interface exists. Return 0 on success,
* PCAP_ERROR_NO_SUCH_DEVICE if the interface doesn't exist,
* or PCAP_ERROR on other failures.
*/
static int
iface_exists(const char *device, char *ebuf)
{
int fd;
struct ifreq ifr;

if (strlen(device) >= sizeof(ifr.ifr_name)) {
ebuf[0] = '\0';
return PCAP_ERROR_NO_SUCH_DEVICE;
}

fd = socket(AF_INET, SOCK_DGRAM, 0);
if (fd < 0) {
pcapint_fmt_errmsg_for_errno(ebuf, PCAP_ERRBUF_SIZE,
errno, "socket");
return PCAP_ERROR;
}

memset(&ifr, 0, sizeof(ifr));
pcapint_strlcpy(ifr.ifr_name, device, sizeof(ifr.ifr_name));

if (ioctl(fd, SIOCGIFINDEX, &ifr) < 0) {
int save_errno = errno;
close(fd);

if (save_errno == ENODEV) {
ebuf[0] = '\0';
return PCAP_ERROR_NO_SUCH_DEVICE;
} else {
pcapint_fmt_errmsg_for_errno(ebuf, PCAP_ERRBUF_SIZE,
save_errno, "SIOCGIFINDEX on %s", device);
return PCAP_ERROR;
}
}

close(fd);
return 0;
}

/*
* Return the index of the given device name. Fill ebuf and return
* -1 on failure.
Expand Down Expand Up @@ -5164,13 +5220,13 @@ iface_get_ts_types(const char *device, pcap_t *handle, char *ebuf)

case ENODEV:
/*
* OK, no such device.
* The user will find that out when they try to
* activate the device; just return an empty
* list of time stamp types.
* No such device.
*
* There's nothing more to say, so clear the
* error message.
*/
handle->tstamp_type_list = NULL;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why tstamp_type_list no longer needs to be set here?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for reviewing these changes
because current code ignoring the ENODEV error and treating it as a success case, assumes the user will discover the real problem later during activation

  case ENODEV:
      ..
      handle->tstamp_type_list = NULL;
      return 0;  // SUCCESS, continue execution

This is problematic because it allows execution to continue with a non existent interface, leading to confusing error messages later

The new code immediately fails when it detects ENODEV:

  case ENODEV:
      ...
      ebuf[0] = '\0';
      return PCAP_ERROR_NO_SUCH_DEVICE;  // MMEDIATE FAILURE

after returning PCAP_ERROR_NO_SUCH_DEVICE, the calling function (pcapint_create_interface) do:

  if (iface_get_ts_types(device, handle, ebuf) == -1) {
      pcap_close(handle);  // Cleans up the handle
      return NULL;         // Function exits immediately
  }

Since the function exits immediately and calls pcap_close(handle), there's no point in setting tstamp_type_list

return 0;
ebuf[0] = '\0';
return PCAP_ERROR_NO_SUCH_DEVICE;

default:
/*
Expand Down