Skip to content
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.

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

ibv_open_device() not called before ibv_free_device_list()? #1300

Open
huzhijiang opened this issue Mar 25, 2024 · 2 comments
Open

ibv_open_device() not called before ibv_free_device_list()? #1300

huzhijiang opened this issue Mar 25, 2024 · 2 comments

Comments

@huzhijiang
Copy link

https://github.com/the-tcpdump-group/libpcap/blob/408e16ce185cb049da5c686d1bce5024cc8f43df/pcap-rdmasniff.c#L425C2-L425C33

According to the verbs.h API comment, Client code must open all devices it intends to use before calling ibv_free_device_list().

@guyharris
Copy link
Member

Submit a pull request.

@infrastation
Copy link
Member

On my Linux host the header /usr/include/infiniband/verbs.h includes the following:

/**
 * ibv_get_device_list - Get list of IB devices currently available
 * @num_devices: optional.  if non-NULL, set to the number of devices
 * returned in the array.
 *
 * Return a NULL-terminated array of IB devices.  The array can be
 * released with ibv_free_device_list().
 */
struct ibv_device **ibv_get_device_list(int *num_devices);

/**
 * ibv_free_device_list - Free list from ibv_get_device_list()
 *
 * Free an array of devices returned from ibv_get_device_list().  Once
 * the array is freed, pointers to devices that were not opened with
 * ibv_open_device() are no longer valid.  Client code must open all
 * devices it intends to use before calling ibv_free_device_list().
 */
void ibv_free_device_list(struct ibv_device **list);

/**
 * ibv_open_device - Initialize device for use
 */
struct ibv_context *ibv_open_device(struct ibv_device *device);

This means one valid way to use the API is to call ibv_get_device_list() first, to call ibv_open_device() N times (where N >= 0) and to call ibv_free_device_list() last. This way ibv_open_device() can make a copy of its input before ibv_free_device_list() frees it.

In the current implementation rdmasniff_create() saves the address (but not the contents) of the matching temporary struct ibv_device and rdmasniff_activate() feeds that pointer to ibv_open_device(), which is a plain use-after-free, so the reporter makes a sound point.

One thing I do not understand yet is whether copying the entire struct ibv_device in rdmasniff_create() would always work as expected. Another possible solution could be to have a helper function to iterate over the device list, to use it once in rdmasniff_create() to find and save the device name and the port number, then in rdmasniff_activate() use it again, but this time opening the device within the loop and returning it.

For reference, the library source code seems to be this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

3 participants