On Fri, Mar 22, 2024 at 04:31:58PM +0100, Arnd Bergmann wrote:
> On Fri, Mar 22, 2024, at 16:09, Günther Noack wrote:
> > From: Mickaël Salaün <mic@xxxxxxxxxxx>
> >
> > Add a new vfs_get_ioctl_handler() helper to identify if an IOCTL command
> > is handled by the first IOCTL layer. Each IOCTL command is now handled
> > by a dedicated function, and all of them use the same signature.
>
> Sorry I didn't already reply the previous time you sent this.
> I don't really like the idea of going through another indirect
> pointer for each of the ioctls here, both because of the
> complexity at the source level, and the potential cost on
> architectures that need heavy barriers around indirect
> function calls.
>
> > -static int ioctl_fibmap(struct file *filp, int __user *p)
> > +static int ioctl_fibmap(struct file *filp, unsigned int fd, unsigned
> > long arg)
> > {
> > + int __user *p = (void __user *)arg;
>
> The new version doesn't seem like an improvement when you
> need the extra type casts here.
>
> As a completely different approach, would it perhaps be
> sufficient to define security_file_ioctl_compat() in a
> way that it may return a special error code signifying
> "don't call into fops->{unlocked,compat}_ioctl"?
>
> This way landlock could trivially allow ioctls on e.g.
> normal file systems, sockets and block devices but prevent
> them on character devices it does not trust.
Thank you for the review, Arnd! I gave your suggestion a shot and it seems
cleaner - I'll send an updated patch shortly.
—Günther
