lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Message-ID: <301ce6ae-f925-a1a5-3be0-06da4ddfafc1@intel.com> Date: Wed, 23 Aug 2023 11:05:14 -0700 From: "Linga, Pavan Kumar" <pavan.kumar.linga@...el.com> To: Jakub Kicinski <kuba@...nel.org> CC: Tony Nguyen <anthony.l.nguyen@...el.com>, <davem@...emloft.net>, <pabeni@...hat.com>, <edumazet@...gle.com>, <netdev@...r.kernel.org>, "Alan Brady" <alan.brady@...el.com>, <emil.s.tantilov@...el.com>, <jesse.brandeburg@...el.com>, <sridhar.samudrala@...el.com>, <shiraz.saleem@...el.com>, <sindhu.devale@...el.com>, <willemb@...gle.com>, <decot@...gle.com>, <andrew@...n.ch>, <leon@...nel.org>, <mst@...hat.com>, <simon.horman@...igine.com>, <shannon.nelson@....com>, <stephen@...workplumber.org>, Alice Michael <alice.michael@...el.com>, Joshua Hay <joshua.a.hay@...el.com>, Phani Burra <phani.r.burra@...el.com> Subject: Re: [PATCH net-next v5 14/15] idpf: add ethtool callbacks On 8/21/2023 2:02 PM, Jakub Kicinski wrote: > On Mon, 21 Aug 2023 13:41:15 -0700 Linga, Pavan Kumar wrote: >> On 8/18/2023 11:58 AM, Jakub Kicinski wrote: >>> On Tue, 15 Aug 2023 17:43:04 -0700 Tony Nguyen wrote: >>>> +static u32 idpf_get_rxfh_indir_size(struct net_device *netdev) >>>> +{ >>>> + struct idpf_vport *vport = idpf_netdev_to_vport(netdev); >>>> + struct idpf_vport_user_config_data *user_config; >>>> + >>>> + if (!vport) >>>> + return -EINVAL; >>> >>> defensive programming? how do we have a netdev and no vport? >> >> During a hardware reset, the control plane will reinitialize its vport >> configuration along with the hardware resources which in turn requires >> the driver to reallocate the vports as well. For this reason the vports >> will be freed, but the netdev will be preserved. > > HW reset path should take appropriate locks so that the normal control > path can't be exposed to transient errors. > > User space will 100% not know what to do with a GET reporting EINVAL. > Agreed, looking into using locks to protect such cases. >>>> + dev = &vport->adapter->pdev->dev; >>>> + if (!(ch->combined_count || (ch->rx_count && ch->tx_count))) { >>>> + dev_err(dev, "Please specify at least 1 Rx and 1 Tx channel\n"); >>> >>> The error msg doesn't seem to fit the second part of the condition. >>> >> >> The negation part is to the complete check which means it takes 0 >> [tx|rx]_count into consideration. > > Ah, missed the negation. In that case I think the check is not needed, > pretty sure core checks this. > My bad. After taking a closer look, the above check is similar compared to that of the core check. Will remove the check as it is redundant. >>>> + return -EINVAL; >>>> + } >>>> + >>>> + num_req_tx_q = ch->combined_count + ch->tx_count; >>>> + num_req_rx_q = ch->combined_count + ch->rx_count; >>>> + >>>> + dev = &vport->adapter->pdev->dev; >>>> + /* It's possible to specify number of queues that exceeds max in a way >>>> + * that stack won't catch for us, this should catch that. >>>> + */ >>> >>> How, tho? >> >> If the user tries to pass the combined along with the txq or rxq values, >> then it is possbile to cross the max supported values. So the following >> checks are needed to protect those cases. Core checks the max values for >> the individual arguments but not the combined + [tx|rx]. > > I see, please add something along those lines to the comment. Sure, will do. Thanks, Pavan
Powered by blists - more mailing lists