Message ID | 5186e815b3d827c8c0f6da2298990a16859a2823.1718131546.git.jacob.martin@canonical.com |
---|---|
State | New |
Headers | show |
Series | idxd: fix NULL pointer dereference reading wq op_config attribute | expand |
On 11.06.24 20:59, Jacob Martin wrote: > BugLink: https://bugs.launchpad.net/bugs/2069081 > > The backport of commit b0325aefd398 ("dmaengine: idxd: add WQ operation > cap restriction support") for K5.15 omitted a line setting the > is_visible callback of idxd_wq_attribute_group to the > idxd_wq_attr_visible function introduced in the same commit. > > This results in the op_config attribute being accessible from userspace > when the underlying wq->opcap_bmap pointer used to service reads from it > is uninitialized, leading to a NULL pointer dereference when the > op_config attribute is read. Resolve this by setting the is_visible > callback as the upstream commit does. > > Signed-off-by: Jacob Martin <jacob.martin@canonical.com> Acked-by: Stefan Bader <stefan.bader@canonical.com> > --- Interesting how I managed to drop that bit. Good catch. I probably would add a standard Fixes: b0325aefd398 ("dmaengine: idxd: add WQ operation cap restriction support") on the s-o-b area. -Stefan > drivers/dma/idxd/sysfs.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/dma/idxd/sysfs.c b/drivers/dma/idxd/sysfs.c > index 0908746ce586..636f349b6e2d 100644 > --- a/drivers/dma/idxd/sysfs.c > +++ b/drivers/dma/idxd/sysfs.c > @@ -1114,6 +1114,7 @@ static umode_t idxd_wq_attr_visible(struct kobject *kobj, > > static const struct attribute_group idxd_wq_attribute_group = { > .attrs = idxd_wq_attributes, > + .is_visible = idxd_wq_attr_visible, > }; > > static const struct attribute_group *idxd_wq_attribute_groups[] = {
On 24/06/12 10:20am, Stefan Bader wrote: > On 11.06.24 20:59, Jacob Martin wrote: > > BugLink: https://bugs.launchpad.net/bugs/2069081 > > > > The backport of commit b0325aefd398 ("dmaengine: idxd: add WQ operation > > cap restriction support") for K5.15 omitted a line setting the > > is_visible callback of idxd_wq_attribute_group to the > > idxd_wq_attr_visible function introduced in the same commit. > > > > This results in the op_config attribute being accessible from userspace > > when the underlying wq->opcap_bmap pointer used to service reads from it > > is uninitialized, leading to a NULL pointer dereference when the > > op_config attribute is read. Resolve this by setting the is_visible > > callback as the upstream commit does. > > > > Signed-off-by: Jacob Martin <jacob.martin@canonical.com> > Acked-by: Stefan Bader <stefan.bader@canonical.com> > > --- > > Interesting how I managed to drop that bit. Good catch. I probably would add > a standard This begs the question of how could we have caught it in the first place? interdiff is a good tool to catch or give hints of unexpected differences between a backport and the original. It would be awesome to have some automation that offers that for each submitted patch.
diff --git a/drivers/dma/idxd/sysfs.c b/drivers/dma/idxd/sysfs.c index 0908746ce586..636f349b6e2d 100644 --- a/drivers/dma/idxd/sysfs.c +++ b/drivers/dma/idxd/sysfs.c @@ -1114,6 +1114,7 @@ static umode_t idxd_wq_attr_visible(struct kobject *kobj, static const struct attribute_group idxd_wq_attribute_group = { .attrs = idxd_wq_attributes, + .is_visible = idxd_wq_attr_visible, }; static const struct attribute_group *idxd_wq_attribute_groups[] = {
BugLink: https://bugs.launchpad.net/bugs/2069081 The backport of commit b0325aefd398 ("dmaengine: idxd: add WQ operation cap restriction support") for K5.15 omitted a line setting the is_visible callback of idxd_wq_attribute_group to the idxd_wq_attr_visible function introduced in the same commit. This results in the op_config attribute being accessible from userspace when the underlying wq->opcap_bmap pointer used to service reads from it is uninitialized, leading to a NULL pointer dereference when the op_config attribute is read. Resolve this by setting the is_visible callback as the upstream commit does. Signed-off-by: Jacob Martin <jacob.martin@canonical.com> --- drivers/dma/idxd/sysfs.c | 1 + 1 file changed, 1 insertion(+)