On Fri, 11 Aug 2023 16:32:28 -0700
Bjorn Andersson <quic_bjorande@xxxxxxxxxxx> wrote:
> On Fri, Aug 11, 2023 at 11:01:50PM +0200, Andrew Lunn wrote:
> > > +static ssize_t qmp_debugfs_write(struct file *file, const char __user *userstr,
> > > + size_t len, loff_t *pos)
> > > +{
> > > + struct qmp *qmp = file->private_data;
> > > + char buf[QMP_MSG_LEN];
> > > + int ret;
> > > +
> > > + if (!len || len >= QMP_MSG_LEN)
> > > + return -EINVAL;
> > > +
> > > + if (copy_from_user(buf, userstr, len))
> > > + return -EFAULT;
> > > + buf[len] = '\0';
> > > +
> > > + ret = qmp_send(qmp, buf);
> > > + if (ret < 0)
> > > + return ret;
> >
> > Sorry, but you still appear to be sending binary blobs from userspace
> > to the firmware. This is not liked.
> >
>
> As mentioned in the cover letter, I do recognize your concern here. I
> don't see it as a realistic way to work around the kernel for reasons of
> being proprietary - given that we don't have debugfs mounted in the vast
> majority of product.
Anyone who cares about security, and has things like kernel lockdown turned on
is going to be scared by this. If you allow API to tell firmware to do any arbitrary thing
it means you could be telling firmware "please read this area of kernel memory for me"
