Current kernel will report NULL pointer dereference with following
back trace:
interrupt_cnt_enable_write from counter_comp_u8_store+0xc0/0xf4
counter_comp_u8_store from dev_attr_store+0x24/0x30
dev_attr_store from sysfs_kf_write+0x48/0x54
sysfs_kf_write from kernfs_fop_write_iter+0x128/0x1c8
kernfs_fop_write_iter from vfs_write+0x124/0x1b4
vfs_write from ksys_write+0x88/0xe0
ksys_write from sys_write+0x18/0x1c
sys_write from ret_fast_syscall+0x0/0x1c
Add missing dev_set_drvdata() to fix it.
Signed-off-by: Oleksij Rempel <[email protected]>
---
drivers/counter/counter-core.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/counter/counter-core.c b/drivers/counter/counter-core.c
index 7e0957eea094..1de16d5e9fbc 100644
--- a/drivers/counter/counter-core.c
+++ b/drivers/counter/counter-core.c
@@ -98,6 +98,8 @@ struct counter_device *counter_alloc(size_t sizeof_priv)
counter = &ch->counter;
dev = &counter->dev;
+ dev_set_drvdata(dev, counter);
+
/* Acquire unique ID */
err = ida_alloc(&counter_ida, GFP_KERNEL);
if (err < 0)
--
2.30.2
On Thu, Feb 03, 2022 at 02:54:18PM +0100, Oleksij Rempel wrote:
> Current kernel will report NULL pointer dereference with following
> back trace:
> interrupt_cnt_enable_write from counter_comp_u8_store+0xc0/0xf4
> counter_comp_u8_store from dev_attr_store+0x24/0x30
> dev_attr_store from sysfs_kf_write+0x48/0x54
> sysfs_kf_write from kernfs_fop_write_iter+0x128/0x1c8
> kernfs_fop_write_iter from vfs_write+0x124/0x1b4
> vfs_write from ksys_write+0x88/0xe0
> ksys_write from sys_write+0x18/0x1c
> sys_write from ret_fast_syscall+0x0/0x1c
>
> Add missing dev_set_drvdata() to fix it.
>
> Signed-off-by: Oleksij Rempel <[email protected]>
Hi Oleksij,
This dev_set_drvdata() was removed in commit b56346ddbd82 ("counter: Use
container_of instead of drvdata to track counter_device"). It looks like
we overlooked the counter-sysfs.c file when we made that change.
Would you instead replace the dev_get_drvdata() calls in counter-sysfs.c
with respective container_of() calls? Add a Fixes tag referencing commit
b56346ddbd82 as well to your commit message.
Thanks,
William Breathitt Gray
> ---
> drivers/counter/counter-core.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/counter/counter-core.c b/drivers/counter/counter-core.c
> index 7e0957eea094..1de16d5e9fbc 100644
> --- a/drivers/counter/counter-core.c
> +++ b/drivers/counter/counter-core.c
> @@ -98,6 +98,8 @@ struct counter_device *counter_alloc(size_t sizeof_priv)
> counter = &ch->counter;
> dev = &counter->dev;
>
> + dev_set_drvdata(dev, counter);
> +
> /* Acquire unique ID */
> err = ida_alloc(&counter_ida, GFP_KERNEL);
> if (err < 0)
> --
> 2.30.2
>
dev_get_drvdata() returns NULL since commit b56346ddbd82 ("counter: Use
container_of instead of drvdata to track counter_device") which wrongly
claimed there were no users of drvdata. Convert to container_of() to
fix a null pointer dereference.
Reported-by: Oleksij Rempel <[email protected]>
Fixes: b56346ddbd82 ("counter: Use container_of instead of drvdata to track counter_device")
Signed-off-by: Uwe Kleine-König <[email protected]>
---
drivers/counter/counter-sysfs.c | 17 +++++++++++------
1 file changed, 11 insertions(+), 6 deletions(-)
diff --git a/drivers/counter/counter-sysfs.c b/drivers/counter/counter-sysfs.c
index 7cc4d1d523ea..04eac41dad33 100644
--- a/drivers/counter/counter-sysfs.c
+++ b/drivers/counter/counter-sysfs.c
@@ -19,6 +19,11 @@
#include "counter-sysfs.h"
+static inline struct counter_device *counter_from_dev(struct device *dev)
+{
+ return container_of(dev, struct counter_device, dev);
+}
+
/**
* struct counter_attribute - Counter sysfs attribute
* @dev_attr: device attribute for sysfs
@@ -90,7 +95,7 @@ static ssize_t counter_comp_u8_show(struct device *dev,
struct device_attribute *attr, char *buf)
{
const struct counter_attribute *const a = to_counter_attribute(attr);
- struct counter_device *const counter = dev_get_drvdata(dev);
+ struct counter_device *const counter = counter_from_dev(dev);
int err;
u8 data = 0;
@@ -122,7 +127,7 @@ static ssize_t counter_comp_u8_store(struct device *dev,
const char *buf, size_t len)
{
const struct counter_attribute *const a = to_counter_attribute(attr);
- struct counter_device *const counter = dev_get_drvdata(dev);
+ struct counter_device *const counter = counter_from_dev(dev);
int err;
bool bool_data = 0;
u8 data = 0;
@@ -158,7 +163,7 @@ static ssize_t counter_comp_u32_show(struct device *dev,
struct device_attribute *attr, char *buf)
{
const struct counter_attribute *const a = to_counter_attribute(attr);
- struct counter_device *const counter = dev_get_drvdata(dev);
+ struct counter_device *const counter = counter_from_dev(dev);
const struct counter_available *const avail = a->comp.priv;
int err;
u32 data = 0;
@@ -221,7 +226,7 @@ static ssize_t counter_comp_u32_store(struct device *dev,
const char *buf, size_t len)
{
const struct counter_attribute *const a = to_counter_attribute(attr);
- struct counter_device *const counter = dev_get_drvdata(dev);
+ struct counter_device *const counter = counter_from_dev(dev);
struct counter_count *const count = a->parent;
struct counter_synapse *const synapse = a->comp.priv;
const struct counter_available *const avail = a->comp.priv;
@@ -281,7 +286,7 @@ static ssize_t counter_comp_u64_show(struct device *dev,
struct device_attribute *attr, char *buf)
{
const struct counter_attribute *const a = to_counter_attribute(attr);
- struct counter_device *const counter = dev_get_drvdata(dev);
+ struct counter_device *const counter = counter_from_dev(dev);
int err;
u64 data = 0;
@@ -309,7 +314,7 @@ static ssize_t counter_comp_u64_store(struct device *dev,
const char *buf, size_t len)
{
const struct counter_attribute *const a = to_counter_attribute(attr);
- struct counter_device *const counter = dev_get_drvdata(dev);
+ struct counter_device *const counter = counter_from_dev(dev);
int err;
u64 data = 0;
base-commit: e783362eb54cd99b2cac8b3a9aeac942e6f6ac07
--
2.34.1
On Thu, Feb 17, 2022 at 04:48:50PM +0200, Jarkko Nikula wrote:
> On 2/8/22 03:42, William Breathitt Gray wrote:
> > On Fri, Feb 04, 2022 at 09:25:56AM +0100, Uwe Kleine-K?nig wrote:
> > > dev_get_drvdata() returns NULL since commit b56346ddbd82 ("counter: Use
> > > container_of instead of drvdata to track counter_device") which wrongly
> > > claimed there were no users of drvdata. Convert to container_of() to
> > > fix a null pointer dereference.
> > >
> > > Reported-by: Oleksij Rempel <[email protected]>
> > > Fixes: b56346ddbd82 ("counter: Use container_of instead of drvdata to track counter_device")
> > > Signed-off-by: Uwe Kleine-K?nig <[email protected]>
> >
> > I'll pick this up and apply it to my tree.
>
> Perhaps late but I hit this same issue, patch here fixes it and I wanted to
> confirm it.
>
> Tested-by: Jarkko Nikula <[email protected]>
I wonder if this patch is scheduled for 5.17. Currently it's not even in
next ... :-\
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-K?nig |
Industrial Linux Solutions | https://www.pengutronix.de/ |
Hi
On 2/8/22 03:42, William Breathitt Gray wrote:
> On Fri, Feb 04, 2022 at 09:25:56AM +0100, Uwe Kleine-König wrote:
>> dev_get_drvdata() returns NULL since commit b56346ddbd82 ("counter: Use
>> container_of instead of drvdata to track counter_device") which wrongly
>> claimed there were no users of drvdata. Convert to container_of() to
>> fix a null pointer dereference.
>>
>> Reported-by: Oleksij Rempel <[email protected]>
>> Fixes: b56346ddbd82 ("counter: Use container_of instead of drvdata to track counter_device")
>> Signed-off-by: Uwe Kleine-König <[email protected]>
>
> I'll pick this up and apply it to my tree.
>
Perhaps late but I hit this same issue, patch here fixes it and I wanted
to confirm it.
Tested-by: Jarkko Nikula <[email protected]>
On Thu, Feb 17, 2022 at 05:03:08PM +0100, Uwe Kleine-König wrote:
> On Thu, Feb 17, 2022 at 04:48:50PM +0200, Jarkko Nikula wrote:
> > On 2/8/22 03:42, William Breathitt Gray wrote:
> > > On Fri, Feb 04, 2022 at 09:25:56AM +0100, Uwe Kleine-König wrote:
> > > > dev_get_drvdata() returns NULL since commit b56346ddbd82 ("counter: Use
> > > > container_of instead of drvdata to track counter_device") which wrongly
> > > > claimed there were no users of drvdata. Convert to container_of() to
> > > > fix a null pointer dereference.
> > > >
> > > > Reported-by: Oleksij Rempel <[email protected]>
> > > > Fixes: b56346ddbd82 ("counter: Use container_of instead of drvdata to track counter_device")
> > > > Signed-off-by: Uwe Kleine-König <[email protected]>
> > >
> > > I'll pick this up and apply it to my tree.
> >
> > Perhaps late but I hit this same issue, patch here fixes it and I wanted to
> > confirm it.
> >
> > Tested-by: Jarkko Nikula <[email protected]>
>
> I wonder if this patch is scheduled for 5.17. Currently it's not even in
> next ... :-\
>
> Best regards
> Uwe
Hi Uwe,
I've got it in my tree. I'm sending a pull request for the Counter
patches for 5.17 next week, so they should all be merged after that.
Sincerely,
William Breathitt Gray
Hello,
On Fri, Feb 18, 2022 at 08:42:51AM +0900, William Breathitt Gray wrote:
> On Thu, Feb 17, 2022 at 05:03:08PM +0100, Uwe Kleine-K?nig wrote:
> > On Thu, Feb 17, 2022 at 04:48:50PM +0200, Jarkko Nikula wrote:
> > > On 2/8/22 03:42, William Breathitt Gray wrote:
> > > > On Fri, Feb 04, 2022 at 09:25:56AM +0100, Uwe Kleine-K?nig wrote:
> > > > > dev_get_drvdata() returns NULL since commit b56346ddbd82 ("counter: Use
> > > > > container_of instead of drvdata to track counter_device") which wrongly
> > > > > claimed there were no users of drvdata. Convert to container_of() to
> > > > > fix a null pointer dereference.
> > > > >
> > > > > Reported-by: Oleksij Rempel <[email protected]>
> > > > > Fixes: b56346ddbd82 ("counter: Use container_of instead of drvdata to track counter_device")
> > > > > Signed-off-by: Uwe Kleine-K?nig <[email protected]>
> > > >
> > > > I'll pick this up and apply it to my tree.
> > >
> > > Perhaps late but I hit this same issue, patch here fixes it and I wanted to
> > > confirm it.
> > >
> > > Tested-by: Jarkko Nikula <[email protected]>
> >
> > I wonder if this patch is scheduled for 5.17. Currently it's not even in
> > next ... :-\
>
> I've got it in my tree. I'm sending a pull request for the Counter
> patches for 5.17 next week, so they should all be merged after that.
That's good. Still I think you could make live easier for your users to
find fixes if your tree was included in next. And in MAINTAINERS.
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-K?nig |
Industrial Linux Solutions | https://www.pengutronix.de/ |
On Fri, Feb 18, 2022 at 08:47:51AM +0100, Uwe Kleine-König wrote:
> Hello,
>
> On Fri, Feb 18, 2022 at 08:42:51AM +0900, William Breathitt Gray wrote:
> > On Thu, Feb 17, 2022 at 05:03:08PM +0100, Uwe Kleine-König wrote:
> > > On Thu, Feb 17, 2022 at 04:48:50PM +0200, Jarkko Nikula wrote:
> > > > On 2/8/22 03:42, William Breathitt Gray wrote:
> > > > > On Fri, Feb 04, 2022 at 09:25:56AM +0100, Uwe Kleine-König wrote:
> > > > > > dev_get_drvdata() returns NULL since commit b56346ddbd82 ("counter: Use
> > > > > > container_of instead of drvdata to track counter_device") which wrongly
> > > > > > claimed there were no users of drvdata. Convert to container_of() to
> > > > > > fix a null pointer dereference.
> > > > > >
> > > > > > Reported-by: Oleksij Rempel <[email protected]>
> > > > > > Fixes: b56346ddbd82 ("counter: Use container_of instead of drvdata to track counter_device")
> > > > > > Signed-off-by: Uwe Kleine-König <[email protected]>
> > > > >
> > > > > I'll pick this up and apply it to my tree.
> > > >
> > > > Perhaps late but I hit this same issue, patch here fixes it and I wanted to
> > > > confirm it.
> > > >
> > > > Tested-by: Jarkko Nikula <[email protected]>
> > >
> > > I wonder if this patch is scheduled for 5.17. Currently it's not even in
> > > next ... :-\
> >
> > I've got it in my tree. I'm sending a pull request for the Counter
> > patches for 5.17 next week, so they should all be merged after that.
>
> That's good. Still I think you could make live easier for your users to
> find fixes if your tree was included in next. And in MAINTAINERS.
>
> Best regards
> Uwe
That's a fair point, I'll update MAINTAINERS and set up my fixes branch
for inclusion in linux-next.
Incidentally, it looks like this is the only fix I have merged; I'll
submit it now by itself so we don't have to wait until next week.
Thanks,
William Breathitt Gray