Hi Dan, On 2/18/21 1:33 PM, Dan Carpenter wrote: > Hi Arnaud, > > url: https://github.com/0day-ci/linux/commits/Arnaud-Pouliquen/introduce-a-generic-IOCTL-interface-for-RPMsg-channels-management/20210217-214044 > base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git f40ddce88593482919761f74910f42f4b84c004b > config: riscv-randconfig-m031-20210215 (attached as .config) > compiler: riscv32-linux-gcc (GCC) 9.3.0 > > If you fix the issue, kindly add following tag as appropriate > Reported-by: kernel test robot <lkp@xxxxxxxxx> > Reported-by: Dan Carpenter <dan.carpenter@xxxxxxxxxx> > > smatch warnings: > drivers/rpmsg/rpmsg_char.c:429 rpmsg_chrdev_probe() error: we previously assumed 'rpdev->ept' could be null (see line 423) > > vim +429 drivers/rpmsg/rpmsg_char.c > > 7337f30f7a4426 Arnaud Pouliquen 2021-02-17 413 static int rpmsg_chrdev_probe(struct rpmsg_device *rpdev) > 7337f30f7a4426 Arnaud Pouliquen 2021-02-17 414 { > 7337f30f7a4426 Arnaud Pouliquen 2021-02-17 415 struct rpmsg_channel_info chinfo; > 7337f30f7a4426 Arnaud Pouliquen 2021-02-17 416 struct rpmsg_eptdev *eptdev; > 7337f30f7a4426 Arnaud Pouliquen 2021-02-17 417 > 7337f30f7a4426 Arnaud Pouliquen 2021-02-17 418 memcpy(chinfo.name, RPMSG_CHAR_DEVNAME, sizeof(RPMSG_CHAR_DEVNAME)); > 7337f30f7a4426 Arnaud Pouliquen 2021-02-17 419 chinfo.src = rpdev->src; > 7337f30f7a4426 Arnaud Pouliquen 2021-02-17 420 chinfo.dst = rpdev->dst; > 7337f30f7a4426 Arnaud Pouliquen 2021-02-17 421 > 7337f30f7a4426 Arnaud Pouliquen 2021-02-17 422 eptdev = __rpmsg_chrdev_create_eptdev(rpdev, &rpdev->dev, chinfo); > 7337f30f7a4426 Arnaud Pouliquen 2021-02-17 @423 if (IS_ERR(eptdev) && rpdev->ept) { > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ > This condition is strange. > > 7337f30f7a4426 Arnaud Pouliquen 2021-02-17 424 rpmsg_destroy_ept(rpdev->ept); > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ > What? Why are we undoing this when it's not something that we created? > This seems like a layering violation... Right,something is not clean here, I need to crosscheck, but should be if (IS_ERR(eptdev) && ) { return PTR_ERR(eptdev); } The endpoint is already destroyed by rpmsg_dev_probe on error. > > 7337f30f7a4426 Arnaud Pouliquen 2021-02-17 425 return PTR_ERR(eptdev); > 7337f30f7a4426 Arnaud Pouliquen 2021-02-17 426 } > 7337f30f7a4426 Arnaud Pouliquen 2021-02-17 427 > 7337f30f7a4426 Arnaud Pouliquen 2021-02-17 428 /* Set the private field of the default endpoint to retrieve context on callback. */ > 7337f30f7a4426 Arnaud Pouliquen 2021-02-17 @429 rpdev->ept->priv = eptdev; > ^^^^^^^^^^^^^^^^^^^^^^^^^ > If "rpdev->ept" is NULL this will Oops. If "eptdev" is an error pointer > that seems wrong as well. rpdev->ept is set in rpmsg_dev_probe as the callback is defined so can not be null, so probably a false positive here. eptdev can not be an error pointer here for the same reason. Anyway adding a check on the pointer, is not a big work and can prevent from future issue. As consequence of you multi-reports I have installed your smatch tool on my PC and added it in my compilation chain. :) Thanks for the review and the tool, Arnaud > > 7337f30f7a4426 Arnaud Pouliquen 2021-02-17 430 > 7337f30f7a4426 Arnaud Pouliquen 2021-02-17 431 return 0; > 7337f30f7a4426 Arnaud Pouliquen 2021-02-17 432 } > > --- > 0-DAY CI Kernel Test Service, Intel Corporation > https://lists.01.org/hyperkitty/list/kbuild-all@xxxxxxxxxxxx >
