| Message ID | 20180529144339.16538-8-mszeredi@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, May 29, 2018 at 04:43:07PM +0200, Miklos Szeredi wrote:
> This is needed by the stacked ioctl implementation in overlayfs.
EXPORT_SYMBOL_GPL for exporting random internals, please. Same
for any following patches.
On Mon, Jun 04, 2018 at 01:49:04AM -0700, Christoph Hellwig wrote: > On Tue, May 29, 2018 at 04:43:07PM +0200, Miklos Szeredi wrote: > > This is needed by the stacked ioctl implementation in overlayfs. > > EXPORT_SYMBOL_GPL for exporting random internals, please. Same > for any following patches. *blink* Christoph, get real and RTFS - vfs_ioctl() simply calls ->unlocked_ioctl(); all there is to it. This isn't even a case of "using that function establishes that the caller is a derived work" - *anyone* who can see definition of file_operations can bloody well open-code it. There isn't anything establishing derivation here. Hell, it could've been a static inline in include/linux/fs.h and it would neither differ from many other inlines in there nor need an export at all. This is really getting close to lxo-worthy levels of bogosity... More interesting question is why do we want to pass those ioctls to layers in the first place, especially if it's something with different availability (or, worse yet, argument layouts) before and after copyup.
On Sun, Jun 10, 2018 at 6:57 AM, Al Viro <viro@zeniv.linux.org.uk> wrote: > On Mon, Jun 04, 2018 at 01:49:04AM -0700, Christoph Hellwig wrote: >> On Tue, May 29, 2018 at 04:43:07PM +0200, Miklos Szeredi wrote: >> > This is needed by the stacked ioctl implementation in overlayfs. >> >> EXPORT_SYMBOL_GPL for exporting random internals, please. Same >> for any following patches. > > *blink* > > Christoph, get real and RTFS - vfs_ioctl() simply calls ->unlocked_ioctl(); > all there is to it. > > This isn't even a case of "using that function establishes that the > caller is a derived work" - *anyone* who can see definition of > file_operations can bloody well open-code it. There isn't anything > establishing derivation here. > > Hell, it could've been a static inline in include/linux/fs.h and it would > neither differ from many other inlines in there nor need an export at all. > > This is really getting close to lxo-worthy levels of bogosity... > > More interesting question is why do we want to pass those ioctls to layers > in the first place, especially if it's something with different availability > (or, worse yet, argument layouts) before and after copyup. We don't. Obviously need to make sure to only ever do ioctl's in overlayfs that have a common definition across filesystems. Not a lot of those, luckily... Thanks, Miklos
On Mon, Jun 11, 2018 at 09:19:01AM +0200, Miklos Szeredi wrote: > We don't. Obviously need to make sure to only ever do ioctl's in > overlayfs that have a common definition across filesystems. Not a lot > of those, luckily... Which are those? If they are common and possibly called from kernel code they should probably be made into methods instead.
On Mon, Jun 11, 2018 at 6:24 PM, Christoph Hellwig <hch@infradead.org> wrote: > On Mon, Jun 11, 2018 at 09:19:01AM +0200, Miklos Szeredi wrote: >> We don't. Obviously need to make sure to only ever do ioctl's in >> overlayfs that have a common definition across filesystems. Not a lot >> of those, luckily... > > Which are those? If they are common and possibly called from kernel > code they should probably be made into methods instead. FS_IOC* Haven't looked deeply. For now overlayfs just implements FS_IOC_{GET|SET}FLAGS because some of these flags are quite generic and implementing them on the overlay is easy. Yes, turning into a method makes sense. Thanks, Miklos
On Tue, Jun 19, 2018 at 04:04:41PM +0200, Miklos Szeredi wrote: > FS_IOC* > > Haven't looked deeply. For now overlayfs just implements > FS_IOC_{GET|SET}FLAGS because some of these flags are quite generic > and implementing them on the overlay is easy. > > Yes, turning into a method makes sense. Do you want to do this or should I send a patch?
On Tue, Jun 19, 2018 at 4:24 PM, Christoph Hellwig <hch@infradead.org> wrote: > On Tue, Jun 19, 2018 at 04:04:41PM +0200, Miklos Szeredi wrote: >> FS_IOC* >> >> Haven't looked deeply. For now overlayfs just implements >> FS_IOC_{GET|SET}FLAGS because some of these flags are quite generic >> and implementing them on the overlay is easy. >> >> Yes, turning into a method makes sense. > > Do you want to do this or should I send a patch? Do it. You are much more familiar with regular fs that implement these ioctls. Untangling overlap between FS_IOC_...FLAGS and FS_IOC_...XATTR looks "interesting". Thanks, Miklos
On Tue, Jun 19, 2018 at 04:34:33PM +0200, Miklos Szeredi wrote: > On Tue, Jun 19, 2018 at 4:24 PM, Christoph Hellwig <hch@infradead.org> wrote: > > On Tue, Jun 19, 2018 at 04:04:41PM +0200, Miklos Szeredi wrote: > >> FS_IOC* > >> > >> Haven't looked deeply. For now overlayfs just implements > >> FS_IOC_{GET|SET}FLAGS because some of these flags are quite generic > >> and implementing them on the overlay is easy. > >> > >> Yes, turning into a method makes sense. > > > > Do you want to do this or should I send a patch? > > Do it. You are much more familiar with regular fs that implement > these ioctls. Untangling overlap between FS_IOC_...FLAGS and > FS_IOC_...XATTR looks "interesting". Suggestion: have that go through ->setattr(); that's what ATTR_ATTR_FLAG was supposed to be for, IIRC.
diff --git a/fs/internal.h b/fs/internal.h index b82725ba3054..6821cf475fc6 100644 --- a/fs/internal.h +++ b/fs/internal.h @@ -189,7 +189,6 @@ extern const struct dentry_operations ns_dentry_operations; */ extern int do_vfs_ioctl(struct file *file, unsigned int fd, unsigned int cmd, unsigned long arg); -extern long vfs_ioctl(struct file *file, unsigned int cmd, unsigned long arg); /* * iomap support: diff --git a/fs/ioctl.c b/fs/ioctl.c index 4823431d1c9d..41071915f411 100644 --- a/fs/ioctl.c +++ b/fs/ioctl.c @@ -49,6 +49,7 @@ long vfs_ioctl(struct file *filp, unsigned int cmd, unsigned long arg) out: return error; } +EXPORT_SYMBOL(vfs_ioctl); static int ioctl_fibmap(struct file *filp, int __user *p) { diff --git a/include/linux/fs.h b/include/linux/fs.h index 1ea3f153b7f8..598c60092c11 100644 --- a/include/linux/fs.h +++ b/include/linux/fs.h @@ -1623,6 +1623,8 @@ int vfs_mkobj(struct dentry *, umode_t, int (*f)(struct dentry *, umode_t, void *), void *); +extern long vfs_ioctl(struct file *file, unsigned int cmd, unsigned long arg); + /* * VFS file helper functions. */
This is needed by the stacked ioctl implementation in overlayfs. Signed-off-by: Miklos Szeredi <mszeredi@redhat.com> --- fs/internal.h | 1 - fs/ioctl.c | 1 + include/linux/fs.h | 2 ++ 3 files changed, 3 insertions(+), 1 deletion(-)