From a93187ced507953654eebd2e608894fc321d2eba Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Tue, 10 Nov 2020 01:04:36 +0900 Subject: [PATCH 1/2] ethtool: add several assertions --- src/shared/ethtool-util.c | 100 +++++++++++++++++++++++++++++--------- 1 file changed, 77 insertions(+), 23 deletions(-) diff --git a/src/shared/ethtool-util.c b/src/shared/ethtool-util.c index e60d516126..bf5d5c6a0c 100644 --- a/src/shared/ethtool-util.c +++ b/src/shared/ethtool-util.c @@ -173,14 +173,18 @@ static int ethtool_connect_or_warn(int *ret, bool warn) { int ethtool_get_driver(int *ethtool_fd, const char *ifname, char **ret) { struct ethtool_drvinfo ecmd = { - .cmd = ETHTOOL_GDRVINFO + .cmd = ETHTOOL_GDRVINFO, }; struct ifreq ifr = { - .ifr_data = (void*) &ecmd + .ifr_data = (void*) &ecmd, }; char *d; int r; + assert(ethtool_fd); + assert(ifname); + assert(ret); + if (*ethtool_fd < 0) { r = ethtool_connect_or_warn(ethtool_fd, true); if (r < 0) @@ -201,9 +205,14 @@ int ethtool_get_driver(int *ethtool_fd, const char *ifname, char **ret) { return 0; } -int ethtool_get_link_info(int *ethtool_fd, const char *ifname, - int *ret_autonegotiation, uint64_t *ret_speed, - Duplex *ret_duplex, NetDevPort *ret_port) { +int ethtool_get_link_info( + int *ethtool_fd, + const char *ifname, + int *ret_autonegotiation, + uint64_t *ret_speed, + Duplex *ret_duplex, + NetDevPort *ret_port) { + struct ethtool_cmd ecmd = { .cmd = ETHTOOL_GSET, }; @@ -212,6 +221,9 @@ int ethtool_get_link_info(int *ethtool_fd, const char *ifname, }; int r; + assert(ethtool_fd); + assert(ifname); + if (*ethtool_fd < 0) { r = ethtool_connect_or_warn(ethtool_fd, false); if (r < 0) @@ -292,14 +304,17 @@ int ethtool_get_permanent_macaddr(int *ethtool_fd, const char *ifname, struct et int ethtool_set_speed(int *ethtool_fd, const char *ifname, unsigned speed, Duplex duplex) { struct ethtool_cmd ecmd = { - .cmd = ETHTOOL_GSET + .cmd = ETHTOOL_GSET, }; struct ifreq ifr = { - .ifr_data = (void*) &ecmd + .ifr_data = (void*) &ecmd, }; bool need_update = false; int r; + assert(ethtool_fd); + assert(ifname); + if (speed == 0 && duplex == _DUP_INVALID) return 0; @@ -350,14 +365,17 @@ int ethtool_set_speed(int *ethtool_fd, const char *ifname, unsigned speed, Duple int ethtool_set_wol(int *ethtool_fd, const char *ifname, WakeOnLan wol) { struct ethtool_wolinfo ecmd = { - .cmd = ETHTOOL_GWOL + .cmd = ETHTOOL_GWOL, }; struct ifreq ifr = { - .ifr_data = (void*) &ecmd + .ifr_data = (void*) &ecmd, }; bool need_update = false; int r; + assert(ethtool_fd); + assert(ifname); + if (wol == _WOL_INVALID) return 0; @@ -439,14 +457,18 @@ int ethtool_set_wol(int *ethtool_fd, const char *ifname, WakeOnLan wol) { int ethtool_set_nic_buffer_size(int *ethtool_fd, const char *ifname, const netdev_ring_param *ring) { struct ethtool_ringparam ecmd = { - .cmd = ETHTOOL_GRINGPARAM + .cmd = ETHTOOL_GRINGPARAM, }; struct ifreq ifr = { - .ifr_data = (void*) &ecmd + .ifr_data = (void*) &ecmd, }; bool need_update = false; int r; + assert(ethtool_fd); + assert(ifname); + assert(ring); + if (*ethtool_fd < 0) { r = ethtool_connect_or_warn(ethtool_fd, true); if (r < 0) @@ -490,7 +512,7 @@ int ethtool_set_nic_buffer_size(int *ethtool_fd, const char *ifname, const netde return 0; } -static int get_stringset(int ethtool_fd, struct ifreq *ifr, int stringset_id, struct ethtool_gstrings **gstrings) { +static int get_stringset(int ethtool_fd, struct ifreq *ifr, int stringset_id, struct ethtool_gstrings **ret) { _cleanup_free_ struct ethtool_gstrings *strings = NULL; struct { struct ethtool_sset_info info; @@ -504,6 +526,10 @@ static int get_stringset(int ethtool_fd, struct ifreq *ifr, int stringset_id, st unsigned len; int r; + assert(ethtool_fd >= 0); + assert(ifr); + assert(ret); + ifr->ifr_data = (void *) &buffer.info; r = ioctl(ethtool_fd, SIOCETHTOOL, ifr); @@ -534,7 +560,7 @@ static int get_stringset(int ethtool_fd, struct ifreq *ifr, int stringset_id, st if (r < 0) return -errno; - *gstrings = TAKE_PTR(strings); + *ret = TAKE_PTR(strings); return 0; } @@ -572,6 +598,10 @@ int ethtool_set_features(int *ethtool_fd, const char *ifname, const int *feature struct ifreq ifr = {}; int i, r; + assert(ethtool_fd); + assert(ifname); + assert(features); + if (*ethtool_fd < 0) { r = ethtool_connect_or_warn(ethtool_fd, true); if (r < 0) @@ -606,7 +636,7 @@ int ethtool_set_features(int *ethtool_fd, const char *ifname, const int *feature return 0; } -static int get_glinksettings(int fd, struct ifreq *ifr, struct ethtool_link_usettings **g) { +static int get_glinksettings(int fd, struct ifreq *ifr, struct ethtool_link_usettings **ret) { struct ecmd { struct ethtool_link_settings req; __u32 link_mode_data[3 * ETHTOOL_LINK_MODE_MASK_MAX_KERNEL_NU32]; @@ -617,6 +647,10 @@ static int get_glinksettings(int fd, struct ifreq *ifr, struct ethtool_link_uset unsigned offset; int r; + assert(fd >= 0); + assert(ifr); + assert(ret); + /* The interaction user/kernel via the new API requires a small ETHTOOL_GLINKSETTINGS handshake first to agree on the length of the link mode bitmaps. If kernel doesn't agree with user, it returns the bitmap length it is expecting from user as a negative @@ -662,18 +696,22 @@ static int get_glinksettings(int fd, struct ifreq *ifr, struct ethtool_link_uset offset += ecmd.req.link_mode_masks_nwords; memcpy(u->link_modes.lp_advertising, &ecmd.link_mode_data[offset], 4 * ecmd.req.link_mode_masks_nwords); - *g = u; + *ret = u; return 0; } -static int get_gset(int fd, struct ifreq *ifr, struct ethtool_link_usettings **u) { +static int get_gset(int fd, struct ifreq *ifr, struct ethtool_link_usettings **ret) { struct ethtool_link_usettings *e; struct ethtool_cmd ecmd = { .cmd = ETHTOOL_GSET, }; int r; + assert(fd >= 0); + assert(ifr); + assert(ret); + ifr->ifr_data = (void *) &ecmd; r = ioctl(fd, SIOCETHTOOL, ifr); @@ -699,7 +737,7 @@ static int get_gset(int fd, struct ifreq *ifr, struct ethtool_link_usettings **u .link_modes.lp_advertising[0] = ecmd.lp_advertising, }; - *u = e; + *ret = e; return 0; } @@ -712,6 +750,10 @@ static int set_slinksettings(int fd, struct ifreq *ifr, const struct ethtool_lin unsigned offset; int r; + assert(fd >= 0); + assert(ifr); + assert(u); + if (u->base.cmd != ETHTOOL_GLINKSETTINGS || u->base.link_mode_masks_nwords <= 0) return -EINVAL; @@ -741,6 +783,10 @@ static int set_sset(int fd, struct ifreq *ifr, const struct ethtool_link_usettin }; int r; + assert(fd >= 0); + assert(ifr); + assert(u); + if (u->base.cmd != ETHTOOL_GSET || u->base.link_mode_masks_nwords <= 0) return -EINVAL; @@ -781,10 +827,13 @@ int ethtool_set_glinksettings( uint64_t speed, Duplex duplex, NetDevPort port) { + _cleanup_free_ struct ethtool_link_usettings *u = NULL; struct ifreq ifr = {}; int r; + assert(fd); + assert(ifname); assert(advertise); if (autonegotiation != AUTONEG_DISABLE && memeqzero(advertise, sizeof(uint32_t) * N_ADVERTISE)) { @@ -838,15 +887,18 @@ int ethtool_set_glinksettings( int ethtool_set_channels(int *fd, const char *ifname, const netdev_channels *channels) { struct ethtool_channels ecmd = { - .cmd = ETHTOOL_GCHANNELS + .cmd = ETHTOOL_GCHANNELS, }; struct ifreq ifr = { - .ifr_data = (void*) &ecmd + .ifr_data = (void*) &ecmd, }; - bool need_update = false; int r; + assert(fd); + assert(ifname); + assert(channels); + if (*fd < 0) { r = ethtool_connect_or_warn(fd, true); if (r < 0) @@ -892,15 +944,17 @@ int ethtool_set_channels(int *fd, const char *ifname, const netdev_channels *cha int ethtool_set_flow_control(int *fd, const char *ifname, int rx, int tx, int autoneg) { struct ethtool_pauseparam ecmd = { - .cmd = ETHTOOL_GPAUSEPARAM + .cmd = ETHTOOL_GPAUSEPARAM, }; struct ifreq ifr = { - .ifr_data = (void*) &ecmd + .ifr_data = (void*) &ecmd, }; - bool need_update = false; int r; + assert(fd); + assert(ifname); + if (*fd < 0) { r = ethtool_connect_or_warn(fd, true); if (r < 0) From 861de64e6858bc92b154ad70d1cee41ae5b75835 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Tue, 10 Nov 2020 01:14:38 +0900 Subject: [PATCH 2/2] ethtool: make ethtool_get_driver() return -ENODATA if ioctl succeeds but driver name is empty Inspired by #17532. --- src/shared/ethtool-util.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/shared/ethtool-util.c b/src/shared/ethtool-util.c index bf5d5c6a0c..e6fab262f2 100644 --- a/src/shared/ethtool-util.c +++ b/src/shared/ethtool-util.c @@ -197,6 +197,9 @@ int ethtool_get_driver(int *ethtool_fd, const char *ifname, char **ret) { if (r < 0) return -errno; + if (isempty(ecmd.driver)) + return -ENODATA; + d = strdup(ecmd.driver); if (!d) return -ENOMEM;