Skip to content
Permalink
Browse files Browse the repository at this point in the history
5421 devzvol_readdir() needs to be more careful with strchr
Reviewed by: Keith Wesolowski <keith.wesolowski@joyent.com>
Reviewed by: Jerry Jelinek <jerry.jelinek@joyent.com>
Approved by: Dan McDonald <danmcd@omniti.com>
  • Loading branch information
rmustacc committed Dec 9, 2014
1 parent 7adb730 commit d656868
Showing 1 changed file with 4 additions and 1 deletion.
5 changes: 4 additions & 1 deletion usr/src/uts/common/fs/dev/sdev_zvolops.c
Expand Up @@ -792,7 +792,10 @@ devzvol_readdir(struct vnode *dvp, struct uio *uiop, struct cred *cred,
return (devname_readdir_func(dvp, uiop, cred, eofp, 0));
}

ptr = strchr(ptr + 1, '/') + 1;
ptr = strchr(ptr + 1, '/');
if (ptr == NULL)
return (ENOENT);
ptr++;
rw_exit(&sdvp->sdev_contents);
sdev_iter_datasets(dvp, ZFS_IOC_DATASET_LIST_NEXT, ptr);
rw_enter(&sdvp->sdev_contents, RW_READER);
Expand Down

5 comments on commit d656868

@myaut
Copy link

@myaut myaut commented on d656868 Dec 9, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't rw_exit(&sdvp->sdev_contents); should be called before return (ENOENT);? It can induce a deadlock...

@tsoome
Copy link
Contributor

@tsoome tsoome commented on d656868 Dec 9, 2014 via email

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@myaut
Copy link

@myaut myaut commented on d656868 Dec 9, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, my mistake. Didn't notice that lock is re-locked again later

@rmustacc
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, importantly look at, for example, getdents64, you'll note that it takes an VOP rwlock before calling VOP_READDIR, and then releases it afterwards. The fact that we're dropping the lock across some of these calls is a bit unfortunate, to be honest, but not the problem here.

@danmcd
Copy link
Member

@danmcd danmcd commented on d656868 Dec 23, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we close this now?

Please sign in to comment.