On Mon, Jul 24, 2023 at 1:25 PM Dan Carpenter <dan.carpenter@xxxxxxxxxx> wrote:
>
> Hello Shyam Prasad N,
>
> The patch 27bfeaa7b929: "cifs: allow dumping keys for directories
> too" from Jun 16, 2023 (linux-next), leads to the following Smatch
> static checker warning:
>
> fs/smb/client/ioctl.c:481 cifs_ioctl()
> error: 'tlink' dereferencing possible ERR_PTR()
>
> fs/smb/client/ioctl.c
> 469 case CIFS_DUMP_FULL_KEY:
> 470 /*
> 471 * Dump encryption keys (handles any key sizes)
> 472 */
> 473 if (pSMBFile == NULL)
> 474 break;
> 475 if (!capable(CAP_SYS_ADMIN)) {
> 476 rc = -EACCES;
> 477 break;
> 478 }
> 479 cifs_sb = CIFS_SB(inode->i_sb);
> 480 tlink = cifs_sb_tlink(cifs_sb);
>
> cifs_sb_tlink() requires error checking.
>
> --> 481 tcon = tlink_tcon(tlink);
> 482 rc = cifs_dump_full_key(tcon, (void __user *)arg);
> 483 cifs_put_tlink(tlink);
> 484 break;
> 485 case CIFS_IOC_NOTIFY:
> 486 if (!S_ISDIR(inode->i_mode)) {
> 487 /* Notify can only be done on directories */
> 488 rc = -EOPNOTSUPP;
> 489 break;
> 490 }
> 491 cifs_sb = CIFS_SB(inode->i_sb);
> 492 tlink = cifs_sb_tlink(cifs_sb);
> 493 if (IS_ERR(tlink)) {
> 494 rc = PTR_ERR(tlink);
> 495 break;
> 496 }
> 497 tcon = tlink_tcon(tlink);
> 498 if (tcon && tcon->ses->server->ops->notify) {
> 499 rc = tcon->ses->server->ops->notify(xid,
> 500 filep, (void __user *)arg,
> 501 false /* no ret data */);
> 502 cifs_dbg(FYI, "ioctl notify rc %d\n", rc);
> 503 } else
> 504 rc = -EOPNOTSUPP;
> 505 cifs_put_tlink(tlink);
> 506 break;
>
> regards,
> dan carpenter
Hi Dan,
Thanks for bringing this to our notice.
I think similar checks are missing in a few other places in the code.
I'll submit a new patch for this soon.
--
Regards,
Shyam
