Message ID | 20200111163743.4339-1-ap420073@gmail.com |
---|---|
State | Changes Requested |
Delegated to: | David Miller |
Headers | show |
Series | netdevsim: fix a several bugs in netdevsim module | expand |
Hi Taehee, url: https://github.com/0day-ci/linux/commits/Taehee-Yoo/netdevsim-fix-a-several-bugs-in-netdevsim-module/20200112-004546 base: https://git.kernel.org/pub/scm/linux/kernel/git/davem/net.git a5c3a7c0ce1a1cfab15404018933775d7222a517 If you fix the issue, kindly add following tag Reported-by: kbuild test robot <lkp@intel.com> Reported-by: Dan Carpenter <dan.carpenter@oracle.com> smatch warnings: drivers/net/netdevsim/bpf.c:246 nsim_bpf_create_prog() error: dereferencing freed memory 'state' # https://github.com/0day-ci/linux/commit/923e31529b0b3f039f837f54c4a1bbd77793256b git remote add linux-review https://github.com/0day-ci/linux git remote update linux-review git checkout 923e31529b0b3f039f837f54c4a1bbd77793256b vim +/state +246 drivers/net/netdevsim/bpf.c d514f41e793d2c Jiri Pirko 2019-04-25 227 static int nsim_bpf_create_prog(struct nsim_dev *nsim_dev, b26b6946a62f37 Jiri Pirko 2019-04-12 228 struct bpf_prog *prog) 31d3ad832948c7 Jakub Kicinski 2017-12-01 229 { 31d3ad832948c7 Jakub Kicinski 2017-12-01 230 struct nsim_bpf_bound_prog *state; 31d3ad832948c7 Jakub Kicinski 2017-12-01 231 char name[16]; 31d3ad832948c7 Jakub Kicinski 2017-12-01 232 31d3ad832948c7 Jakub Kicinski 2017-12-01 233 state = kzalloc(sizeof(*state), GFP_KERNEL); 31d3ad832948c7 Jakub Kicinski 2017-12-01 234 if (!state) 31d3ad832948c7 Jakub Kicinski 2017-12-01 235 return -ENOMEM; 31d3ad832948c7 Jakub Kicinski 2017-12-01 236 d514f41e793d2c Jiri Pirko 2019-04-25 237 state->nsim_dev = nsim_dev; 31d3ad832948c7 Jakub Kicinski 2017-12-01 238 state->prog = prog; 31d3ad832948c7 Jakub Kicinski 2017-12-01 239 state->state = "verify"; 31d3ad832948c7 Jakub Kicinski 2017-12-01 240 31d3ad832948c7 Jakub Kicinski 2017-12-01 241 /* Program id is not populated yet when we create the state. */ d514f41e793d2c Jiri Pirko 2019-04-25 242 sprintf(name, "%u", nsim_dev->prog_id_gen++); d514f41e793d2c Jiri Pirko 2019-04-25 243 state->ddir = debugfs_create_dir(name, nsim_dev->ddir_bpf_bound_progs); 923e31529b0b3f Taehee Yoo 2020-01-11 244 if (IS_ERR(state->ddir)) { 31d3ad832948c7 Jakub Kicinski 2017-12-01 245 kfree(state); ^^^^^ state is freed. 923e31529b0b3f Taehee Yoo 2020-01-11 @246 return PTR_ERR(state->ddir); ^^^^^^^^^^^ Then dereferenced afterward. 31d3ad832948c7 Jakub Kicinski 2017-12-01 247 } 31d3ad832948c7 Jakub Kicinski 2017-12-01 248 31d3ad832948c7 Jakub Kicinski 2017-12-01 249 debugfs_create_u32("id", 0400, state->ddir, &prog->aux->id); 31d3ad832948c7 Jakub Kicinski 2017-12-01 250 debugfs_create_file("state", 0400, state->ddir, 31d3ad832948c7 Jakub Kicinski 2017-12-01 251 &state->state, &nsim_bpf_string_fops); 31d3ad832948c7 Jakub Kicinski 2017-12-01 252 debugfs_create_bool("loaded", 0400, state->ddir, &state->is_loaded); 31d3ad832948c7 Jakub Kicinski 2017-12-01 253 d514f41e793d2c Jiri Pirko 2019-04-25 254 list_add_tail(&state->l, &nsim_dev->bpf_bound_progs); 31d3ad832948c7 Jakub Kicinski 2017-12-01 255 31d3ad832948c7 Jakub Kicinski 2017-12-01 256 prog->aux->offload->dev_priv = state; 31d3ad832948c7 Jakub Kicinski 2017-12-01 257 31d3ad832948c7 Jakub Kicinski 2017-12-01 258 return 0; 31d3ad832948c7 Jakub Kicinski 2017-12-01 259 } --- 0-DAY kernel test infrastructure Open Source Technology Center https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org Intel Corporation
On Fri, 17 Jan 2020 at 12:36, Dan Carpenter <dan.carpenter@oracle.com> wrote: > Hi Dan, > Hi Taehee, > > url: https://github.com/0day-ci/linux/commits/Taehee-Yoo/netdevsim-fix-a-several-bugs-in-netdevsim-module/20200112-004546 > base: https://git.kernel.org/pub/scm/linux/kernel/git/davem/net.git a5c3a7c0ce1a1cfab15404018933775d7222a517 > > If you fix the issue, kindly add following tag > Reported-by: kbuild test robot <lkp@intel.com> > Reported-by: Dan Carpenter <dan.carpenter@oracle.com> > > smatch warnings: > drivers/net/netdevsim/bpf.c:246 nsim_bpf_create_prog() error: dereferencing freed memory 'state' > > # https://github.com/0day-ci/linux/commit/923e31529b0b3f039f837f54c4a1bbd77793256b > git remote add linux-review https://github.com/0day-ci/linux > git remote update linux-review > git checkout 923e31529b0b3f039f837f54c4a1bbd77793256b > vim +/state +246 drivers/net/netdevsim/bpf.c > > d514f41e793d2c Jiri Pirko 2019-04-25 227 static int nsim_bpf_create_prog(struct nsim_dev *nsim_dev, > b26b6946a62f37 Jiri Pirko 2019-04-12 228 struct bpf_prog *prog) > 31d3ad832948c7 Jakub Kicinski 2017-12-01 229 { > 31d3ad832948c7 Jakub Kicinski 2017-12-01 230 struct nsim_bpf_bound_prog *state; > 31d3ad832948c7 Jakub Kicinski 2017-12-01 231 char name[16]; > 31d3ad832948c7 Jakub Kicinski 2017-12-01 232 > 31d3ad832948c7 Jakub Kicinski 2017-12-01 233 state = kzalloc(sizeof(*state), GFP_KERNEL); > 31d3ad832948c7 Jakub Kicinski 2017-12-01 234 if (!state) > 31d3ad832948c7 Jakub Kicinski 2017-12-01 235 return -ENOMEM; > 31d3ad832948c7 Jakub Kicinski 2017-12-01 236 > d514f41e793d2c Jiri Pirko 2019-04-25 237 state->nsim_dev = nsim_dev; > 31d3ad832948c7 Jakub Kicinski 2017-12-01 238 state->prog = prog; > 31d3ad832948c7 Jakub Kicinski 2017-12-01 239 state->state = "verify"; > 31d3ad832948c7 Jakub Kicinski 2017-12-01 240 > 31d3ad832948c7 Jakub Kicinski 2017-12-01 241 /* Program id is not populated yet when we create the state. */ > d514f41e793d2c Jiri Pirko 2019-04-25 242 sprintf(name, "%u", nsim_dev->prog_id_gen++); > d514f41e793d2c Jiri Pirko 2019-04-25 243 state->ddir = debugfs_create_dir(name, nsim_dev->ddir_bpf_bound_progs); > 923e31529b0b3f Taehee Yoo 2020-01-11 244 if (IS_ERR(state->ddir)) { > 31d3ad832948c7 Jakub Kicinski 2017-12-01 245 kfree(state); > ^^^^^ > state is freed. > > 923e31529b0b3f Taehee Yoo 2020-01-11 @246 return PTR_ERR(state->ddir); > ^^^^^^^^^^^ > Then dereferenced afterward. > Thank you for catching this bug. I will fix this. Thank you! Taehee Yoo
diff --git a/drivers/net/netdevsim/bpf.c b/drivers/net/netdevsim/bpf.c index 13e17c82d71c..8d2e157f6efc 100644 --- a/drivers/net/netdevsim/bpf.c +++ b/drivers/net/netdevsim/bpf.c @@ -241,9 +241,9 @@ static int nsim_bpf_create_prog(struct nsim_dev *nsim_dev, /* Program id is not populated yet when we create the state. */ sprintf(name, "%u", nsim_dev->prog_id_gen++); state->ddir = debugfs_create_dir(name, nsim_dev->ddir_bpf_bound_progs); - if (IS_ERR_OR_NULL(state->ddir)) { + if (IS_ERR(state->ddir)) { kfree(state); - return -ENOMEM; + return PTR_ERR(state->ddir); } debugfs_create_u32("id", 0400, state->ddir, &prog->aux->id); @@ -598,8 +598,8 @@ int nsim_bpf_dev_init(struct nsim_dev *nsim_dev) nsim_dev->ddir_bpf_bound_progs = debugfs_create_dir("bpf_bound_progs", nsim_dev->ddir); - if (IS_ERR_OR_NULL(nsim_dev->ddir_bpf_bound_progs)) - return -ENOMEM; + if (IS_ERR(nsim_dev->ddir_bpf_bound_progs)) + return PTR_ERR(nsim_dev->ddir_bpf_bound_progs); nsim_dev->bpf_dev = bpf_offload_dev_create(&nsim_bpf_dev_ops, nsim_dev); err = PTR_ERR_OR_ZERO(nsim_dev->bpf_dev); diff --git a/drivers/net/netdevsim/dev.c b/drivers/net/netdevsim/dev.c index a0c80a70bb23..9ea283a02bcf 100644 --- a/drivers/net/netdevsim/dev.c +++ b/drivers/net/netdevsim/dev.c @@ -92,11 +92,11 @@ static int nsim_dev_debugfs_init(struct nsim_dev *nsim_dev) sprintf(dev_ddir_name, DRV_NAME "%u", nsim_dev->nsim_bus_dev->dev.id); nsim_dev->ddir = debugfs_create_dir(dev_ddir_name, nsim_dev_ddir); - if (IS_ERR_OR_NULL(nsim_dev->ddir)) - return PTR_ERR_OR_ZERO(nsim_dev->ddir) ?: -EINVAL; + if (IS_ERR(nsim_dev->ddir)) + return PTR_ERR(nsim_dev->ddir); nsim_dev->ports_ddir = debugfs_create_dir("ports", nsim_dev->ddir); - if (IS_ERR_OR_NULL(nsim_dev->ports_ddir)) - return PTR_ERR_OR_ZERO(nsim_dev->ports_ddir) ?: -EINVAL; + if (IS_ERR(nsim_dev->ports_ddir)) + return PTR_ERR(nsim_dev->ports_ddir); debugfs_create_bool("fw_update_status", 0600, nsim_dev->ddir, &nsim_dev->fw_update_status); debugfs_create_u32("max_macs", 0600, nsim_dev->ddir, @@ -127,8 +127,8 @@ static int nsim_dev_port_debugfs_init(struct nsim_dev *nsim_dev, sprintf(port_ddir_name, "%u", nsim_dev_port->port_index); nsim_dev_port->ddir = debugfs_create_dir(port_ddir_name, nsim_dev->ports_ddir); - if (IS_ERR_OR_NULL(nsim_dev_port->ddir)) - return -ENOMEM; + if (IS_ERR(nsim_dev_port->ddir)) + return PTR_ERR(nsim_dev_port->ddir); sprintf(dev_link_name, "../../../" DRV_NAME "%u", nsim_dev->nsim_bus_dev->dev.id); @@ -943,8 +943,8 @@ int nsim_dev_port_del(struct nsim_bus_dev *nsim_bus_dev, int nsim_dev_init(void) { nsim_dev_ddir = debugfs_create_dir(DRV_NAME, NULL); - if (IS_ERR_OR_NULL(nsim_dev_ddir)) - return -ENOMEM; + if (IS_ERR(nsim_dev_ddir)) + return PTR_ERR(nsim_dev_ddir); return 0; } diff --git a/drivers/net/netdevsim/health.c b/drivers/net/netdevsim/health.c index 9aa637d162eb..30595b1299bd 100644 --- a/drivers/net/netdevsim/health.c +++ b/drivers/net/netdevsim/health.c @@ -285,8 +285,8 @@ int nsim_dev_health_init(struct nsim_dev *nsim_dev, struct devlink *devlink) } health->ddir = debugfs_create_dir("health", nsim_dev->ddir); - if (IS_ERR_OR_NULL(health->ddir)) { - err = PTR_ERR_OR_ZERO(health->ddir) ?: -EINVAL; + if (IS_ERR(health->ddir)) { + err = PTR_ERR(health->ddir); goto err_dummy_reporter_destroy; }
Debugfs APIs return valid pointer or error pointer. it doesn't return NULL. So, using IS_ERR is enough, not using IS_ERR_OR_NULL. Signed-off-by: Taehee Yoo <ap420073@gmail.com> --- drivers/net/netdevsim/bpf.c | 8 ++++---- drivers/net/netdevsim/dev.c | 16 ++++++++-------- drivers/net/netdevsim/health.c | 4 ++-- 3 files changed, 14 insertions(+), 14 deletions(-)