| Message ID | 1415810308-24243-1-git-send-email-martink@posteo.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Nov 12, 2014 at 11:38 AM, Martin Kepplinger <martink@posteo.de> wrote: > The miscdevice core now sets file->private_data to the struct > miscdevice > so don't fail when this is not NULL. > > Signed-off-by: Martin Kepplinger <martink@posteo.de> > --- > This is a question: what does this check provide and does overwriting > file->private_data make any difference? > > Is miscdevice's open() by the user not allowed here, if > file->private_data > is set? > > thanks!! Btrfs uses this in the transaction start ioctl to record the transaction handle being started. Ceph is the main user of the ioctl, and we could setup a hash table if needed. But which call path in miscdevice is doing this? With your patch in place, btrfs would end up overwriting the miscdevice private_data field, which would probably cause problems. -chris -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Am 2014-11-12 um 18:59 schrieb Chris Mason: > On Wed, Nov 12, 2014 at 11:38 AM, Martin Kepplinger <martink@posteo.de> > wrote: >> The miscdevice core now sets file->private_data to the struct miscdevice >> so don't fail when this is not NULL. >> >> Signed-off-by: Martin Kepplinger <martink@posteo.de> >> --- >> This is a question: what does this check provide and does overwriting >> file->private_data make any difference? >> >> Is miscdevice's open() by the user not allowed here, if >> file->private_data >> is set? >> >> thanks!! > > Btrfs uses this in the transaction start ioctl to record the transaction > handle being started. Ceph is the main user of the ioctl, and we could > setup a hash table if needed. But which call path in miscdevice is > doing this? > > With your patch in place, btrfs would end up overwriting the miscdevice > private_data field, which would probably cause problems. > > -chris > I think i was mistaken, sorry. misc_open() used to set file->private_data _only_ if you use set .open in struct file_operations. In current -next this changed and file->private_data is set to struct miscdevice on a (userspace's) open call (misc_open()) just in any case. You do set .open so this wouldn't affect you and this patch can be ignored. martin -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Nov 12, 2014 at 07:34:50PM +0100, Martin Kepplinger wrote: > > Btrfs uses this in the transaction start ioctl to record the transaction > > handle being started. Ceph is the main user of the ioctl, and we could > > setup a hash table if needed. But which call path in miscdevice is > > doing this? > > > > With your patch in place, btrfs would end up overwriting the miscdevice > > private_data field, which would probably cause problems. > > > > -chris > > > > I think i was mistaken, sorry. misc_open() used to set > file->private_data _only_ if you use set .open in struct file_operations. > > In current -next this changed and file->private_data is set to struct > miscdevice on a (userspace's) open call (misc_open()) just in any case. > > You do set .open so this wouldn't affect you and this patch can be ignored. More to the point, this function is not reachable from anything in file_operations of any miscdevice. btrfs_ioctl != btrfs_control_ioctl... -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c index 4399f0c..066ce41 100644 --- a/fs/btrfs/ioctl.c +++ b/fs/btrfs/ioctl.c @@ -3752,10 +3752,6 @@ static long btrfs_ioctl_trans_start(struct file *file) if (!capable(CAP_SYS_ADMIN)) goto out; - ret = -EINPROGRESS; - if (file->private_data) - goto out; - ret = -EROFS; if (btrfs_root_readonly(root)) goto out;
The miscdevice core now sets file->private_data to the struct miscdevice so don't fail when this is not NULL. Signed-off-by: Martin Kepplinger <martink@posteo.de> --- This is a question: what does this check provide and does overwriting file->private_data make any difference? Is miscdevice's open() by the user not allowed here, if file->private_data is set? thanks!! fs/btrfs/ioctl.c | 4 ---- 1 file changed, 4 deletions(-)