On Mon, Jul 24, 2023 at 2:02 PM Shyam Prasad N <nspmangalore@xxxxxxxxx> wrote:
>
> 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
My bad. Only this change is missing the check.
Added that and submitted a new patch. Please check [PATCH] cifs: add
missing return value check for cifs_sb_tlink
--
Regards,
Shyam
