| Message ID | 20240828174340.593130-2-djahchankoike@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net-next,v4] ethtool: pse-pd: move pse validation into set | expand |
Hi Diogo, On Wed, 28 Aug 2024 14:43:17 -0300 Diogo Jahchan Koike <djahchankoike@gmail.com> wrote: > Move validation into set, removing .set_validate operation as its current > implementation holds the rtnl lock for acquiring the PHY device, defeating > the intended purpose of checking before grabbing the lock. [...] > static int > -ethnl_set_pse_validate(struct ethnl_req_info *req_info, struct genl_info *info) > +ethnl_set_pse_validate(struct phy_device *phydev, struct genl_info *info) > { > struct nlattr **tb = info->attrs; > - struct phy_device *phydev; > - > - phydev = ethnl_req_get_phydev(req_info, tb[ETHTOOL_A_PSE_HEADER], > - info->extack); > + ^^^ You have an extra white space here. Make sure you run your patch through the checkpatch script to detect this kind of issues. The rest looks good to me. For the next version, Reviewed-by: Maxime Chevallier <maxime.chevallier@bootlin.com> Thanks, Maxime
Hi Maxime, On 8/29/24 05:35, Maxime Chevallier wrote > You have an extra white space here. Make sure you run your patch through > the checkpatch script to detect this kind of issues. My apologies, I did forget to run checkpatch, thanks for pointing that out. Will send the corrected patch. Thanks, Diogo Jahchan Koike
diff --git a/net/ethtool/pse-pd.c b/net/ethtool/pse-pd.c index 507cb21d6bf0..b56b79f41a3a 100644 --- a/net/ethtool/pse-pd.c +++ b/net/ethtool/pse-pd.c @@ -222,13 +222,10 @@ const struct nla_policy ethnl_pse_set_policy[ETHTOOL_A_PSE_MAX + 1] = { }; static int -ethnl_set_pse_validate(struct ethnl_req_info *req_info, struct genl_info *info) +ethnl_set_pse_validate(struct phy_device *phydev, struct genl_info *info) { struct nlattr **tb = info->attrs; - struct phy_device *phydev; - - phydev = ethnl_req_get_phydev(req_info, tb[ETHTOOL_A_PSE_HEADER], - info->extack); + if (IS_ERR_OR_NULL(phydev)) { NL_SET_ERR_MSG(info->extack, "No PHY is attached"); return -EOPNOTSUPP; @@ -254,7 +251,7 @@ ethnl_set_pse_validate(struct ethnl_req_info *req_info, struct genl_info *info) return -EOPNOTSUPP; } - return 1; + return 0; } static int @@ -262,12 +259,13 @@ ethnl_set_pse(struct ethnl_req_info *req_info, struct genl_info *info) { struct nlattr **tb = info->attrs; struct phy_device *phydev; - int ret = 0; + int ret; phydev = ethnl_req_get_phydev(req_info, tb[ETHTOOL_A_PSE_HEADER], info->extack); - if (IS_ERR_OR_NULL(phydev)) - return -ENODEV; + ret = ethnl_set_pse_validate(phydev, info); + if (ret) + return ret; if (tb[ETHTOOL_A_C33_PSE_AVAIL_PW_LIMIT]) { unsigned int pw_limit; @@ -314,7 +312,6 @@ const struct ethnl_request_ops ethnl_pse_request_ops = { .fill_reply = pse_fill_reply, .cleanup_data = pse_cleanup_data, - .set_validate = ethnl_set_pse_validate, .set = ethnl_set_pse, /* PSE has no notification */ };
Move validation into set, removing .set_validate operation as its current implementation holds the rtnl lock for acquiring the PHY device, defeating the intended purpose of checking before grabbing the lock. Reported-by: syzbot+ec369e6d58e210135f71@syzkaller.appspotmail.com Closes: https://syzkaller.appspot.com/bug?extid=ec369e6d58e210135f71 Fixes: 31748765bed3 ("net: ethtool: pse-pd: Target the command to the requested PHY") Signed-off-by: Diogo Jahchan Koike <djahchankoike@gmail.com> --- v4: - remove .set_validate op, move validation to set v3: https://lore.kernel.org/all/20240826173913.7763-1-djahchankoike@gmail.com/ - hold rtnl lock while interacting with the PHY device - rollback to v1 v2: https://lore.kernel.org/all/20240826140628.99849-1-djahchankoike@gmail.com/ v1: https://lore.kernel.org/all/20240826130712.91391-1-djahchankoike@gmail.com/ --- net/ethtool/pse-pd.c | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-)