[PATCH 0/2] cdev partition fix

[PATCH 0/2] cdev partition fix

[Sascha Hauer]

cdev partitions are cdevs themselves, but they are handled internally in
the devfs-core. cdev partitions should not be passed as context pointers
to the cdev ops though, because the drivers might do a container_of() on
them to retrieve their driver data. Pass the original cdev the driver
has registered as context to the ops

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
Sascha Hauer (2):
      cdev: pass master cdev to cdev ops
      nvmem: Drop now unnecessary partition quirk

 drivers/nvmem/core.c | 10 ++--------
 fs/devfs-core.c      | 53 ++++++++++++++++++++++++++++++++++++++++------------
 2 files changed, 43 insertions(+), 20 deletions(-)
---
base-commit: 812b498be4057c39d017cad6bf43664fe04ee44b
change-id: 20250602-cdev-part-fixes-e2416b848e82

Best regards,
-- 
Sascha Hauer <s.hauer@pengutronix.de>

[PATCH 1/2] cdev: pass master cdev to cdev ops

[Sascha Hauer]

cdev partitions are cdevs themselves, but they are handled internally in
the devfs-core. cdev partitions should not be passed as context pointers
to the cdev ops though, because the drivers might do a container_of() on
them to retrieve their driver data. Pass the original cdev the driver
has registered as context to the ops. With this we can drop some quirks
in the nvmem core in the next step.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 fs/devfs-core.c | 53 +++++++++++++++++++++++++++++++++++++++++------------
 1 file changed, 41 insertions(+), 12 deletions(-)

diff --git a/fs/devfs-core.c b/fs/devfs-core.c
index 5fafcaecc70d63287e0b716cc0133198c28e79e9..73a13be7336df1eb4e24e8a936986b6069f3783c 100644
--- a/fs/devfs-core.c
+++ b/fs/devfs-core.c
@@ -224,12 +224,21 @@ int cdev_find_free_index(const char *basename)
 	return -EBUSY;	/* all indexes are used */
 }
 
+static struct cdev *cdev_get_master(struct cdev *cdev)
+{
+	while (cdev && cdev_is_partition(cdev))
+		cdev = cdev->master;
+
+	return cdev;
+}
+
 int cdev_open(struct cdev *cdev, unsigned long flags)
 {
+	struct cdev *master = cdev_get_master(cdev);
 	int ret;
 
 	if (cdev->ops->open) {
-		ret = cdev->ops->open(cdev, flags);
+		ret = cdev->ops->open(master, flags);
 		if (ret)
 			return ret;
 	}
@@ -275,8 +284,10 @@ struct cdev *cdev_open_by_name(const char *name, unsigned long flags)
 
 int cdev_close(struct cdev *cdev)
 {
+	struct cdev *master = cdev_get_master(cdev);
+
 	if (cdev->ops->close) {
-		int ret = cdev->ops->close(cdev);
+		int ret = cdev->ops->close(master);
 		if (ret)
 			return ret;
 	}
@@ -288,50 +299,61 @@ int cdev_close(struct cdev *cdev)
 
 ssize_t cdev_read(struct cdev *cdev, void *buf, size_t count, loff_t offset, ulong flags)
 {
+	struct cdev *master = cdev_get_master(cdev);
+
 	if (!cdev->ops->read)
 		return -ENOSYS;
 
-	return cdev->ops->read(cdev, buf, count, cdev->offset +offset, flags);
+	return cdev->ops->read(master, buf, count, cdev->offset +offset, flags);
 }
 
 ssize_t cdev_write(struct cdev *cdev, const void *buf, size_t count, loff_t offset, ulong flags)
 {
+	struct cdev *master = cdev_get_master(cdev);
+
 	if (!cdev->ops->write)
 		return -ENOSYS;
 
-	return cdev->ops->write(cdev, buf, count, cdev->offset + offset, flags);
+	return cdev->ops->write(master, buf, count, cdev->offset + offset, flags);
 }
 
 int cdev_flush(struct cdev *cdev)
 {
+	struct cdev *master = cdev_get_master(cdev);
+
 	if (!cdev->ops->flush)
 		return 0;
 
-	return cdev->ops->flush(cdev);
+	return cdev->ops->flush(master);
 }
 
 int cdev_ioctl(struct cdev *cdev, unsigned int request, void *buf)
 {
+	struct cdev *master = cdev_get_master(cdev);
+
 	if (!cdev->ops->ioctl)
 		return -EINVAL;
 
-	return cdev->ops->ioctl(cdev, request, buf);
+	return cdev->ops->ioctl(master, request, buf);
 }
 
 int cdev_erase(struct cdev *cdev, loff_t count, loff_t offset)
 {
+	struct cdev *master = cdev_get_master(cdev);
+
 	if (!cdev->ops->erase)
 		return -ENOSYS;
 
-	return cdev->ops->erase(cdev, count, cdev->offset + offset);
+	return cdev->ops->erase(master, count, cdev->offset + offset);
 }
 
 int cdev_lseek(struct cdev *cdev, loff_t pos)
 {
+	struct cdev *master = cdev_get_master(cdev);
 	int ret;
 
 	if (cdev->ops->lseek) {
-		ret = cdev->ops->lseek(cdev, pos + cdev->offset);
+		ret = cdev->ops->lseek(master, pos + cdev->offset);
 		if (ret < 0)
 			return ret;
 	}
@@ -341,14 +363,18 @@ int cdev_lseek(struct cdev *cdev, loff_t pos)
 
 int cdev_protect(struct cdev *cdev, size_t count, loff_t offset, int prot)
 {
+	struct cdev *master = cdev_get_master(cdev);
+
 	if (!cdev->ops->protect)
 		return -ENOSYS;
 
-	return cdev->ops->protect(cdev, count, offset + cdev->offset, prot);
+	return cdev->ops->protect(master, count, offset + cdev->offset, prot);
 }
 
 int cdev_discard_range(struct cdev *cdev, loff_t count, loff_t offset)
 {
+	struct cdev *master = cdev_get_master(cdev);
+
 	if (!cdev->ops->discard_range)
 		return -ENOSYS;
 
@@ -361,17 +387,18 @@ int cdev_discard_range(struct cdev *cdev, loff_t count, loff_t offset)
 	if (count + offset > cdev->size)
 		count = cdev->size - offset;
 
-	return cdev->ops->discard_range(cdev, count, offset + cdev->offset);
+	return cdev->ops->discard_range(master, count, offset + cdev->offset);
 }
 
 int cdev_memmap(struct cdev *cdev, void **map, int flags)
 {
+	struct cdev *master = cdev_get_master(cdev);
 	int ret = -ENOSYS;
 
 	if (!cdev->ops->memmap)
 		return -EINVAL;
 
-	ret = cdev->ops->memmap(cdev, map, flags);
+	ret = cdev->ops->memmap(master, map, flags);
 
 	if (!ret)
 		*map = (void *)((unsigned long)*map + (unsigned long)cdev->offset);
@@ -381,8 +408,10 @@ int cdev_memmap(struct cdev *cdev, void **map, int flags)
 
 int cdev_truncate(struct cdev *cdev, size_t size)
 {
+	struct cdev *master = cdev_get_master(cdev);
+
 	if (cdev->ops->truncate)
-		return cdev->ops->truncate(cdev, size);
+		return cdev->ops->truncate(master, size);
 
 	return -EPERM;
 }

-- 
2.39.5

[PATCH 2/2] nvmem: Drop now unnecessary partition quirk

[Sascha Hauer]

The nvmem core uses container_of() on the cdev to retrieve its driver
data. For partitions the nvmem core tried to retrieve the master cdev
first. This is no longer necessary since the devfs-core now passes the
master cdev as context pointer and not the partition cdev.

This also fixes a bug when a partition was partitioned again. In this
case we would have to iterate upwards until we find the master cdev
instead of doing only one level.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/nvmem/core.c | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
index 38dfb2cf2d1f5dcd49be0d24f8c9e16f5963507b..fd3c39fd8e431e8ec392feede933986306e0484a 100644
--- a/drivers/nvmem/core.c
+++ b/drivers/nvmem/core.c
@@ -61,10 +61,7 @@ static ssize_t nvmem_cdev_read(struct cdev *cdev, void *buf, size_t count,
 	struct nvmem_device *nvmem;
 	ssize_t retlen;
 
-	if (cdev->master)
-		nvmem = container_of(cdev->master, struct nvmem_device, cdev);
-	else
-		nvmem = container_of(cdev, struct nvmem_device, cdev);
+	nvmem = container_of(cdev, struct nvmem_device, cdev);
 
 	dev_dbg(cdev->dev, "read ofs: 0x%08llx count: 0x%08zx\n",
 		offset, count);
@@ -80,10 +77,7 @@ static ssize_t nvmem_cdev_write(struct cdev *cdev, const void *buf, size_t count
 	struct nvmem_device *nvmem;
 	ssize_t retlen;
 
-	if (cdev->master)
-		nvmem = container_of(cdev->master, struct nvmem_device, cdev);
-	else
-		nvmem = container_of(cdev, struct nvmem_device, cdev);
+	nvmem = container_of(cdev, struct nvmem_device, cdev);
 
 	dev_dbg(cdev->dev, "write ofs: 0x%08llx count: 0x%08zx\n",
 		offset, count);

-- 
2.39.5

Re: [PATCH 1/2] cdev: pass master cdev to cdev ops

[Ahmad Fatoum]

Hello Sascha,

On 6/2/25 10:51, Sascha Hauer wrote:
> cdev partitions are cdevs themselves, but they are handled internally in
> the devfs-core. cdev partitions should not be passed as context pointers
> to the cdev ops though, because the drivers might do a container_of() on
> them to retrieve their driver data. Pass the original cdev the driver
> has registered as context to the ops. With this we can drop some quirks
> in the nvmem core in the next step.

Given that read/write and so on take absolute offsets, I agree that this
is a sensible change.

> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>

With below nit addressed:

Reviewed-by: Ahmad Fatoum <a.fatoum@pengutronix.de>

> ---
>  fs/devfs-core.c | 53 +++++++++++++++++++++++++++++++++++++++++------------
>  1 file changed, 41 insertions(+), 12 deletions(-)
> 
> diff --git a/fs/devfs-core.c b/fs/devfs-core.c
> index 5fafcaecc70d63287e0b716cc0133198c28e79e9..73a13be7336df1eb4e24e8a936986b6069f3783c 100644
> --- a/fs/devfs-core.c
> +++ b/fs/devfs-core.c
> @@ -224,12 +224,21 @@ int cdev_find_free_index(const char *basename)
>  	return -EBUSY;	/* all indexes are used */
>  }
>  
> +static struct cdev *cdev_get_master(struct cdev *cdev)
> +{
> +	while (cdev && cdev_is_partition(cdev))
> +		cdev = cdev->master;

This can be made into a simple if condition as we do not support links
to links, see cdev_readlink.

Thanks,
Ahmad

> +
> +	return cdev;
> +}
> +
>  int cdev_open(struct cdev *cdev, unsigned long flags)
>  {
> +	struct cdev *master = cdev_get_master(cdev);
>  	int ret;
>  
>  	if (cdev->ops->open) {
> -		ret = cdev->ops->open(cdev, flags);
> +		ret = cdev->ops->open(master, flags);
>  		if (ret)
>  			return ret;
>  	}
> @@ -275,8 +284,10 @@ struct cdev *cdev_open_by_name(const char *name, unsigned long flags)
>  
>  int cdev_close(struct cdev *cdev)
>  {
> +	struct cdev *master = cdev_get_master(cdev);
> +
>  	if (cdev->ops->close) {
> -		int ret = cdev->ops->close(cdev);
> +		int ret = cdev->ops->close(master);
>  		if (ret)
>  			return ret;
>  	}
> @@ -288,50 +299,61 @@ int cdev_close(struct cdev *cdev)
>  
>  ssize_t cdev_read(struct cdev *cdev, void *buf, size_t count, loff_t offset, ulong flags)
>  {
> +	struct cdev *master = cdev_get_master(cdev);
> +
>  	if (!cdev->ops->read)
>  		return -ENOSYS;
>  
> -	return cdev->ops->read(cdev, buf, count, cdev->offset +offset, flags);
> +	return cdev->ops->read(master, buf, count, cdev->offset +offset, flags);
>  }
>  
>  ssize_t cdev_write(struct cdev *cdev, const void *buf, size_t count, loff_t offset, ulong flags)
>  {
> +	struct cdev *master = cdev_get_master(cdev);
> +
>  	if (!cdev->ops->write)
>  		return -ENOSYS;
>  
> -	return cdev->ops->write(cdev, buf, count, cdev->offset + offset, flags);
> +	return cdev->ops->write(master, buf, count, cdev->offset + offset, flags);
>  }
>  
>  int cdev_flush(struct cdev *cdev)
>  {
> +	struct cdev *master = cdev_get_master(cdev);
> +
>  	if (!cdev->ops->flush)
>  		return 0;
>  
> -	return cdev->ops->flush(cdev);
> +	return cdev->ops->flush(master);
>  }
>  
>  int cdev_ioctl(struct cdev *cdev, unsigned int request, void *buf)
>  {
> +	struct cdev *master = cdev_get_master(cdev);
> +
>  	if (!cdev->ops->ioctl)
>  		return -EINVAL;
>  
> -	return cdev->ops->ioctl(cdev, request, buf);
> +	return cdev->ops->ioctl(master, request, buf);
>  }
>  
>  int cdev_erase(struct cdev *cdev, loff_t count, loff_t offset)
>  {
> +	struct cdev *master = cdev_get_master(cdev);
> +
>  	if (!cdev->ops->erase)
>  		return -ENOSYS;
>  
> -	return cdev->ops->erase(cdev, count, cdev->offset + offset);
> +	return cdev->ops->erase(master, count, cdev->offset + offset);
>  }
>  
>  int cdev_lseek(struct cdev *cdev, loff_t pos)
>  {
> +	struct cdev *master = cdev_get_master(cdev);
>  	int ret;
>  
>  	if (cdev->ops->lseek) {
> -		ret = cdev->ops->lseek(cdev, pos + cdev->offset);
> +		ret = cdev->ops->lseek(master, pos + cdev->offset);
>  		if (ret < 0)
>  			return ret;
>  	}
> @@ -341,14 +363,18 @@ int cdev_lseek(struct cdev *cdev, loff_t pos)
>  
>  int cdev_protect(struct cdev *cdev, size_t count, loff_t offset, int prot)
>  {
> +	struct cdev *master = cdev_get_master(cdev);
> +
>  	if (!cdev->ops->protect)
>  		return -ENOSYS;
>  
> -	return cdev->ops->protect(cdev, count, offset + cdev->offset, prot);
> +	return cdev->ops->protect(master, count, offset + cdev->offset, prot);
>  }
>  
>  int cdev_discard_range(struct cdev *cdev, loff_t count, loff_t offset)
>  {
> +	struct cdev *master = cdev_get_master(cdev);
> +
>  	if (!cdev->ops->discard_range)
>  		return -ENOSYS;
>  
> @@ -361,17 +387,18 @@ int cdev_discard_range(struct cdev *cdev, loff_t count, loff_t offset)
>  	if (count + offset > cdev->size)
>  		count = cdev->size - offset;
>  
> -	return cdev->ops->discard_range(cdev, count, offset + cdev->offset);
> +	return cdev->ops->discard_range(master, count, offset + cdev->offset);
>  }
>  
>  int cdev_memmap(struct cdev *cdev, void **map, int flags)
>  {
> +	struct cdev *master = cdev_get_master(cdev);
>  	int ret = -ENOSYS;
>  
>  	if (!cdev->ops->memmap)
>  		return -EINVAL;
>  
> -	ret = cdev->ops->memmap(cdev, map, flags);
> +	ret = cdev->ops->memmap(master, map, flags);
>  
>  	if (!ret)
>  		*map = (void *)((unsigned long)*map + (unsigned long)cdev->offset);
> @@ -381,8 +408,10 @@ int cdev_memmap(struct cdev *cdev, void **map, int flags)
>  
>  int cdev_truncate(struct cdev *cdev, size_t size)
>  {
> +	struct cdev *master = cdev_get_master(cdev);
> +
>  	if (cdev->ops->truncate)
> -		return cdev->ops->truncate(cdev, size);
> +		return cdev->ops->truncate(master, size);
>  
>  	return -EPERM;
>  }
> 

-- 
Pengutronix e.K.                  |                             |
Steuerwalder Str. 21              | http://www.pengutronix.de/  |
31137 Hildesheim, Germany         | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686  | Fax:   +49-5121-206917-5555 |

Re: [PATCH 1/2] cdev: pass master cdev to cdev ops

[Sascha Hauer]

On Mon, Jun 02, 2025 at 11:03:56AM +0200, Ahmad Fatoum wrote:
> Hello Sascha,
> 
> On 6/2/25 10:51, Sascha Hauer wrote:
> > cdev partitions are cdevs themselves, but they are handled internally in
> > the devfs-core. cdev partitions should not be passed as context pointers
> > to the cdev ops though, because the drivers might do a container_of() on
> > them to retrieve their driver data. Pass the original cdev the driver
> > has registered as context to the ops. With this we can drop some quirks
> > in the nvmem core in the next step.
> 
> Given that read/write and so on take absolute offsets, I agree that this
> is a sensible change.
> 
> > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> 
> With below nit addressed:
> 
> Reviewed-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> 
> > ---
> >  fs/devfs-core.c | 53 +++++++++++++++++++++++++++++++++++++++++------------
> >  1 file changed, 41 insertions(+), 12 deletions(-)
> > 
> > diff --git a/fs/devfs-core.c b/fs/devfs-core.c
> > index 5fafcaecc70d63287e0b716cc0133198c28e79e9..73a13be7336df1eb4e24e8a936986b6069f3783c 100644
> > --- a/fs/devfs-core.c
> > +++ b/fs/devfs-core.c
> > @@ -224,12 +224,21 @@ int cdev_find_free_index(const char *basename)
> >  	return -EBUSY;	/* all indexes are used */
> >  }
> >  
> > +static struct cdev *cdev_get_master(struct cdev *cdev)
> > +{
> > +	while (cdev && cdev_is_partition(cdev))
> > +		cdev = cdev->master;
> 
> This can be made into a simple if condition as we do not support links
> to links, see cdev_readlink.

This patch is not about links, it's about partitions and we can indeed
partitiion a partition.

You can try with

addpart /dev/imx-ocotp 128@0x10(foo)
addpart /dev/imx-ocotp.foo 64@0x10(foo1)

reading from the latter fails with a core dump without this patch.

As far as I can tell links are not affected by this problem as we always
resolve links by doing a cdev = cdev_readlink(cdev), so the link cdevs
are never passed down to drivers.

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH 1/2] cdev: pass master cdev to cdev ops

[Ahmad Fatoum]

Hi Sascha,

On 6/2/25 12:12, Sascha Hauer wrote:
> On Mon, Jun 02, 2025 at 11:03:56AM +0200, Ahmad Fatoum wrote:
>> Hello Sascha,
>>
>> On 6/2/25 10:51, Sascha Hauer wrote:
>>> cdev partitions are cdevs themselves, but they are handled internally in
>>> the devfs-core. cdev partitions should not be passed as context pointers
>>> to the cdev ops though, because the drivers might do a container_of() on
>>> them to retrieve their driver data. Pass the original cdev the driver
>>> has registered as context to the ops. With this we can drop some quirks
>>> in the nvmem core in the next step.
>>
>> Given that read/write and so on take absolute offsets, I agree that this
>> is a sensible change.
>>
>>> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
>>
>> With below nit addressed:
>>
>> Reviewed-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
>>
>>> ---
>>>  fs/devfs-core.c | 53 +++++++++++++++++++++++++++++++++++++++++------------
>>>  1 file changed, 41 insertions(+), 12 deletions(-)
>>>
>>> diff --git a/fs/devfs-core.c b/fs/devfs-core.c
>>> index 5fafcaecc70d63287e0b716cc0133198c28e79e9..73a13be7336df1eb4e24e8a936986b6069f3783c 100644
>>> --- a/fs/devfs-core.c
>>> +++ b/fs/devfs-core.c
>>> @@ -224,12 +224,21 @@ int cdev_find_free_index(const char *basename)
>>>  	return -EBUSY;	/* all indexes are used */
>>>  }
>>>  
>>> +static struct cdev *cdev_get_master(struct cdev *cdev)
>>> +{
>>> +	while (cdev && cdev_is_partition(cdev))
>>> +		cdev = cdev->master;
>>
>> This can be made into a simple if condition as we do not support links
>> to links, see cdev_readlink.
> 
> This patch is not about links, it's about partitions and we can indeed
> partitiion a partition.
> 
> You can try with
> 
> addpart /dev/imx-ocotp 128@0x10(foo)
> addpart /dev/imx-ocotp.foo 64@0x10(foo1)
> 
> reading from the latter fails with a core dump without this patch.
> 
> As far as I can tell links are not affected by this problem as we always
> resolve links by doing a cdev = cdev_readlink(cdev), so the link cdevs
> are never passed down to drivers.

Thanks for clarification. You can collect my R-B.

Cheers,
Ahmad

> 
> Sascha
> 

-- 
Pengutronix e.K.                  |                             |
Steuerwalder Str. 21              | http://www.pengutronix.de/  |
31137 Hildesheim, Germany         | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686  | Fax:   +49-5121-206917-5555 |

Re: [PATCH 0/2] cdev partition fix

[Sascha Hauer]

On Mon, 02 Jun 2025 10:51:30 +0200, Sascha Hauer wrote:
> cdev partitions are cdevs themselves, but they are handled internally in
> the devfs-core. cdev partitions should not be passed as context pointers
> to the cdev ops though, because the drivers might do a container_of() on
> them to retrieve their driver data. Pass the original cdev the driver
> has registered as context to the ops
> 
> 
> [...]

Applied, thanks!

[1/2] cdev: pass master cdev to cdev ops
      https://git.pengutronix.de/cgit/barebox/commit/?id=08e276fbe412 (link may not be stable)
[2/2] nvmem: Drop now unnecessary partition quirk
      https://git.pengutronix.de/cgit/barebox/commit/?id=df7bfd5b4fe4 (link may not be stable)

Best regards,
-- 
Sascha Hauer <s.hauer@pengutronix.de>