* [PATCH v1 0/2] soc: qcom: rmtfs-mem: Support non-root rmtfs daemons @ 2018-12-18 0:08 Evan Green 2018-12-18 0:08 ` [PATCH v1 1/2] soc: qcom: rmtfs-mem: Add class to enable uevents Evan Green 2018-12-18 0:08 ` [PATCH v1 2/2] soc: qcom: rmtfs-mem: Make sysfs attributes world-readable Evan Green 0 siblings, 2 replies; 6+ messages in thread From: Evan Green @ 2018-12-18 0:08 UTC (permalink / raw) To: Andy Gross, Bjorn Andersson Cc: Brian Norris, Ben Chan, Evan Green, linux-arm-msm, David Brown, linux-kernel This series contains minor fixes needed to better support running an rmtfs daemon from an unprivileged process: - Enable uevents on the child character device by adding a struct class to the parent. I needed these so that I could change the ownership of /dev/qcom_rmtfs_mem1, and drove myself crazy trying to understand why my udev rules never fired. - Enable access to phys_addr and size sysfs attributes. The daemon needs to read these, and they don't really contain anything sensitive, so expose them. I still need CAP_NET_ADMIN to be able to bind to the right qrtr port, but at least with these changes I can run as a different user, and drop all other privileges. Evan Green (2): soc: qcom: rmtfs-mem: Add class to enable uevents soc: qcom: rmtfs-mem: Make sysfs attributes world-readable drivers/soc/qcom/rmtfs_mem.c | 29 +++++++++++++++++++++-------- 1 file changed, 21 insertions(+), 8 deletions(-) -- 2.18.1 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v1 1/2] soc: qcom: rmtfs-mem: Add class to enable uevents 2018-12-18 0:08 [PATCH v1 0/2] soc: qcom: rmtfs-mem: Support non-root rmtfs daemons Evan Green @ 2018-12-18 0:08 ` Evan Green 2018-12-21 1:18 ` Brian Norris 2018-12-18 0:08 ` [PATCH v1 2/2] soc: qcom: rmtfs-mem: Make sysfs attributes world-readable Evan Green 1 sibling, 1 reply; 6+ messages in thread From: Evan Green @ 2018-12-18 0:08 UTC (permalink / raw) To: Andy Gross, Bjorn Andersson Cc: Brian Norris, Ben Chan, Evan Green, linux-arm-msm, David Brown, linux-kernel Currently the qcom_rmtfs_memN devices are entirely invisible to the udev world. Add a class to the rmtfs device so that uevents fire when the device is added. Signed-off-by: Evan Green <evgreen@chromium.org> --- drivers/soc/qcom/rmtfs_mem.c | 23 ++++++++++++++++++----- 1 file changed, 18 insertions(+), 5 deletions(-) diff --git a/drivers/soc/qcom/rmtfs_mem.c b/drivers/soc/qcom/rmtfs_mem.c index 97bb5989aa211..0bf800ee2a978 100644 --- a/drivers/soc/qcom/rmtfs_mem.c +++ b/drivers/soc/qcom/rmtfs_mem.c @@ -132,6 +132,11 @@ static int qcom_rmtfs_mem_release(struct inode *inode, struct file *filp) return 0; } +static struct class rmtfs_class = { + .owner = THIS_MODULE, + .name = "rmtfs", +}; + static const struct file_operations qcom_rmtfs_mem_fops = { .owner = THIS_MODULE, .open = qcom_rmtfs_mem_open, @@ -173,9 +178,15 @@ static int qcom_rmtfs_mem_probe(struct platform_device *pdev) } + ret = class_register(&rmtfs_class); + if (ret) + return ret; + rmtfs_mem = kzalloc(sizeof(*rmtfs_mem), GFP_KERNEL); - if (!rmtfs_mem) - return -ENOMEM; + if (!rmtfs_mem) { + ret = -ENOMEM; + goto unregister_class; + } rmtfs_mem->addr = rmem->base; rmtfs_mem->client_id = client_id; @@ -199,8 +210,8 @@ static int qcom_rmtfs_mem_probe(struct platform_device *pdev) dev_set_name(&rmtfs_mem->dev, "qcom_rmtfs_mem%d", client_id); rmtfs_mem->dev.id = client_id; + rmtfs_mem->dev.class = &rmtfs_class; rmtfs_mem->dev.devt = MKDEV(MAJOR(qcom_rmtfs_mem_major), client_id); - ret = cdev_device_add(&rmtfs_mem->cdev, &rmtfs_mem->dev); if (ret) { dev_err(&pdev->dev, "failed to add cdev: %d\n", ret); @@ -235,11 +246,13 @@ static int qcom_rmtfs_mem_probe(struct platform_device *pdev) return 0; + remove_cdev: cdev_device_del(&rmtfs_mem->cdev, &rmtfs_mem->dev); put_device: put_device(&rmtfs_mem->dev); - +unregister_class: + class_unregister(&rmtfs_class); return ret; } @@ -258,7 +271,7 @@ static int qcom_rmtfs_mem_remove(struct platform_device *pdev) cdev_device_del(&rmtfs_mem->cdev, &rmtfs_mem->dev); put_device(&rmtfs_mem->dev); - + class_unregister(&rmtfs_class); return 0; } -- 2.18.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v1 1/2] soc: qcom: rmtfs-mem: Add class to enable uevents 2018-12-18 0:08 ` [PATCH v1 1/2] soc: qcom: rmtfs-mem: Add class to enable uevents Evan Green @ 2018-12-21 1:18 ` Brian Norris 2018-12-21 17:56 ` Evan Green 0 siblings, 1 reply; 6+ messages in thread From: Brian Norris @ 2018-12-21 1:18 UTC (permalink / raw) To: Evan Green Cc: Andy Gross, Bjorn Andersson, Ben Chan, linux-arm-msm, David Brown, linux-kernel Hi Evan, On Mon, Dec 17, 2018 at 04:08:33PM -0800, Evan Green wrote: > Currently the qcom_rmtfs_memN devices are entirely invisible to the udev world. > Add a class to the rmtfs device so that uevents fire when the device is added. > > Signed-off-by: Evan Green <evgreen@chromium.org> > --- > > drivers/soc/qcom/rmtfs_mem.c | 23 ++++++++++++++++++----- > 1 file changed, 18 insertions(+), 5 deletions(-) > > diff --git a/drivers/soc/qcom/rmtfs_mem.c b/drivers/soc/qcom/rmtfs_mem.c > index 97bb5989aa211..0bf800ee2a978 100644 > --- a/drivers/soc/qcom/rmtfs_mem.c > +++ b/drivers/soc/qcom/rmtfs_mem.c > @@ -132,6 +132,11 @@ static int qcom_rmtfs_mem_release(struct inode *inode, struct file *filp) > return 0; > } > > +static struct class rmtfs_class = { > + .owner = THIS_MODULE, > + .name = "rmtfs", > +}; > + > static const struct file_operations qcom_rmtfs_mem_fops = { > .owner = THIS_MODULE, > .open = qcom_rmtfs_mem_open, > @@ -173,9 +178,15 @@ static int qcom_rmtfs_mem_probe(struct platform_device *pdev) > > } > > + ret = class_register(&rmtfs_class); > + if (ret) > + return ret; Hmm, is this how classes are supposed to work? Usually, you have the possibility of more than 1 device per class, and therefore you don't register the class in the driver probe -- you register it in the init() routine, or something similar. As it is, I expect this will break if there were ever a second rmtfs device. Brian > + > rmtfs_mem = kzalloc(sizeof(*rmtfs_mem), GFP_KERNEL); > - if (!rmtfs_mem) > - return -ENOMEM; > + if (!rmtfs_mem) { > + ret = -ENOMEM; > + goto unregister_class; > + } > > rmtfs_mem->addr = rmem->base; > rmtfs_mem->client_id = client_id; > @@ -199,8 +210,8 @@ static int qcom_rmtfs_mem_probe(struct platform_device *pdev) > > dev_set_name(&rmtfs_mem->dev, "qcom_rmtfs_mem%d", client_id); > rmtfs_mem->dev.id = client_id; > + rmtfs_mem->dev.class = &rmtfs_class; > rmtfs_mem->dev.devt = MKDEV(MAJOR(qcom_rmtfs_mem_major), client_id); > - > ret = cdev_device_add(&rmtfs_mem->cdev, &rmtfs_mem->dev); > if (ret) { > dev_err(&pdev->dev, "failed to add cdev: %d\n", ret); > @@ -235,11 +246,13 @@ static int qcom_rmtfs_mem_probe(struct platform_device *pdev) > > return 0; > > + > remove_cdev: > cdev_device_del(&rmtfs_mem->cdev, &rmtfs_mem->dev); > put_device: > put_device(&rmtfs_mem->dev); > - > +unregister_class: > + class_unregister(&rmtfs_class); > return ret; > } > > @@ -258,7 +271,7 @@ static int qcom_rmtfs_mem_remove(struct platform_device *pdev) > > cdev_device_del(&rmtfs_mem->cdev, &rmtfs_mem->dev); > put_device(&rmtfs_mem->dev); > - > + class_unregister(&rmtfs_class); > return 0; > } > > -- > 2.18.1 > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v1 1/2] soc: qcom: rmtfs-mem: Add class to enable uevents 2018-12-21 1:18 ` Brian Norris @ 2018-12-21 17:56 ` Evan Green 0 siblings, 0 replies; 6+ messages in thread From: Evan Green @ 2018-12-21 17:56 UTC (permalink / raw) To: Brian Norris Cc: Andy Gross, Bjorn Andersson, Ben Chan, linux-arm-msm, David Brown, linux-kernel On Thu, Dec 20, 2018 at 5:19 PM Brian Norris <briannorris@chromium.org> wrote: > > Hi Evan, > > On Mon, Dec 17, 2018 at 04:08:33PM -0800, Evan Green wrote: > > Currently the qcom_rmtfs_memN devices are entirely invisible to the udev world. > > Add a class to the rmtfs device so that uevents fire when the device is added. > > > > Signed-off-by: Evan Green <evgreen@chromium.org> > > --- > > > > drivers/soc/qcom/rmtfs_mem.c | 23 ++++++++++++++++++----- > > 1 file changed, 18 insertions(+), 5 deletions(-) > > > > diff --git a/drivers/soc/qcom/rmtfs_mem.c b/drivers/soc/qcom/rmtfs_mem.c > > index 97bb5989aa211..0bf800ee2a978 100644 > > --- a/drivers/soc/qcom/rmtfs_mem.c > > +++ b/drivers/soc/qcom/rmtfs_mem.c > > @@ -132,6 +132,11 @@ static int qcom_rmtfs_mem_release(struct inode *inode, struct file *filp) > > return 0; > > } > > > > +static struct class rmtfs_class = { > > + .owner = THIS_MODULE, > > + .name = "rmtfs", > > +}; > > + > > static const struct file_operations qcom_rmtfs_mem_fops = { > > .owner = THIS_MODULE, > > .open = qcom_rmtfs_mem_open, > > @@ -173,9 +178,15 @@ static int qcom_rmtfs_mem_probe(struct platform_device *pdev) > > > > } > > > > + ret = class_register(&rmtfs_class); > > + if (ret) > > + return ret; > > Hmm, is this how classes are supposed to work? Usually, you have the > possibility of more than 1 device per class, and therefore you don't > register the class in the driver probe -- you register it in the init() > routine, or something similar. As it is, I expect this will break if > there were ever a second rmtfs device. > Fair enough. I'll move this to an init routine. ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v1 2/2] soc: qcom: rmtfs-mem: Make sysfs attributes world-readable 2018-12-18 0:08 [PATCH v1 0/2] soc: qcom: rmtfs-mem: Support non-root rmtfs daemons Evan Green 2018-12-18 0:08 ` [PATCH v1 1/2] soc: qcom: rmtfs-mem: Add class to enable uevents Evan Green @ 2018-12-18 0:08 ` Evan Green 2018-12-21 1:21 ` Brian Norris 1 sibling, 1 reply; 6+ messages in thread From: Evan Green @ 2018-12-18 0:08 UTC (permalink / raw) To: Andy Gross, Bjorn Andersson Cc: Brian Norris, Ben Chan, Evan Green, linux-arm-msm, David Brown, linux-kernel In order to run an rmtfs daemon as an unprivileged user, that user would need access to the phys_addr and size sysfs attributes. Sharing these attributes with unprivileged users doesn't really leak anything sensitive, since if you have access to physical memory, the jig is up anyway. Make those attributes readable by all. Signed-off-by: Evan Green <evgreen@chromium.org> --- drivers/soc/qcom/rmtfs_mem.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/soc/qcom/rmtfs_mem.c b/drivers/soc/qcom/rmtfs_mem.c index 0bf800ee2a978..4eeb9f02e7889 100644 --- a/drivers/soc/qcom/rmtfs_mem.c +++ b/drivers/soc/qcom/rmtfs_mem.c @@ -45,9 +45,9 @@ static ssize_t qcom_rmtfs_mem_show(struct device *dev, struct device_attribute *attr, char *buf); -static DEVICE_ATTR(phys_addr, 0400, qcom_rmtfs_mem_show, NULL); -static DEVICE_ATTR(size, 0400, qcom_rmtfs_mem_show, NULL); -static DEVICE_ATTR(client_id, 0400, qcom_rmtfs_mem_show, NULL); +static DEVICE_ATTR(phys_addr, 0444, qcom_rmtfs_mem_show, NULL); +static DEVICE_ATTR(size, 0444, qcom_rmtfs_mem_show, NULL); +static DEVICE_ATTR(client_id, 0444, qcom_rmtfs_mem_show, NULL); static ssize_t qcom_rmtfs_mem_show(struct device *dev, struct device_attribute *attr, -- 2.18.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v1 2/2] soc: qcom: rmtfs-mem: Make sysfs attributes world-readable 2018-12-18 0:08 ` [PATCH v1 2/2] soc: qcom: rmtfs-mem: Make sysfs attributes world-readable Evan Green @ 2018-12-21 1:21 ` Brian Norris 0 siblings, 0 replies; 6+ messages in thread From: Brian Norris @ 2018-12-21 1:21 UTC (permalink / raw) To: Evan Green Cc: Andy Gross, Bjorn Andersson, Ben Chan, linux-arm-msm, David Brown, linux-kernel On Mon, Dec 17, 2018 at 04:08:34PM -0800, Evan Green wrote: > In order to run an rmtfs daemon as an unprivileged user, that user would > need access to the phys_addr and size sysfs attributes. Sharing these > attributes with unprivileged users doesn't really leak anything > sensitive, since if you have access to physical memory, the jig is > up anyway. > > Make those attributes readable by all. > > Signed-off-by: Evan Green <evgreen@chromium.org> > --- Seems fine to me. Reviewed-by: Brian Norris <briannorris@chromium.org> > > drivers/soc/qcom/rmtfs_mem.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/soc/qcom/rmtfs_mem.c b/drivers/soc/qcom/rmtfs_mem.c > index 0bf800ee2a978..4eeb9f02e7889 100644 > --- a/drivers/soc/qcom/rmtfs_mem.c > +++ b/drivers/soc/qcom/rmtfs_mem.c > @@ -45,9 +45,9 @@ static ssize_t qcom_rmtfs_mem_show(struct device *dev, > struct device_attribute *attr, > char *buf); > > -static DEVICE_ATTR(phys_addr, 0400, qcom_rmtfs_mem_show, NULL); > -static DEVICE_ATTR(size, 0400, qcom_rmtfs_mem_show, NULL); > -static DEVICE_ATTR(client_id, 0400, qcom_rmtfs_mem_show, NULL); > +static DEVICE_ATTR(phys_addr, 0444, qcom_rmtfs_mem_show, NULL); > +static DEVICE_ATTR(size, 0444, qcom_rmtfs_mem_show, NULL); > +static DEVICE_ATTR(client_id, 0444, qcom_rmtfs_mem_show, NULL); > > static ssize_t qcom_rmtfs_mem_show(struct device *dev, > struct device_attribute *attr, > -- > 2.18.1 > ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2018-12-21 17:57 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2018-12-18 0:08 [PATCH v1 0/2] soc: qcom: rmtfs-mem: Support non-root rmtfs daemons Evan Green 2018-12-18 0:08 ` [PATCH v1 1/2] soc: qcom: rmtfs-mem: Add class to enable uevents Evan Green 2018-12-21 1:18 ` Brian Norris 2018-12-21 17:56 ` Evan Green 2018-12-18 0:08 ` [PATCH v1 2/2] soc: qcom: rmtfs-mem: Make sysfs attributes world-readable Evan Green 2018-12-21 1:21 ` Brian Norris
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox