Re: [net v1] devlink: fix xa_alloc_cyclic error handling
From: Andrew Lunn <andrew@lunn.ch>
Date: 2025-02-14 13:44:53
On Fri, Feb 14, 2025 at 02:24:53PM +0100, Michal Swiatkowski wrote:
Pierre Riteau [off-list ref] found suspicious handling an error from xa_alloc_cyclic() in scheduler code [1]. The same is done in devlink_rel_alloc().
If the same bug exists twice it might exist more times. Did you find this instance by searching the whole tree? Or just networking? This is also something which would be good to have the static analysers check for. I wounder if smatch can check this? Andrew
quoted hunk ↗ jump to hunk
In case of returning 1 from xa_alloc_cyclic() (wrapping) ERR_PTR(1) will be returned, which will cause IS_ERR() to be false. Which can lead to dereference not allocated pointer (rel). Fix it by checking if err is lower than zero. This wasn't found in real usecase, only noticed. Credit to Pierre. Signed-off-by: Michal Swiatkowski <redacted> --- net/devlink/core.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)diff --git a/net/devlink/core.c b/net/devlink/core.c index f49cd83f1955..7203c39532fc 100644 --- a/net/devlink/core.c +++ b/net/devlink/core.c@@ -117,7 +117,7 @@ static struct devlink_rel *devlink_rel_alloc(void) err = xa_alloc_cyclic(&devlink_rels, &rel->index, rel, xa_limit_32b, &next, GFP_KERNEL); - if (err) { + if (err < 0) { kfree(rel); return ERR_PTR(err); }-- 2.42.0