Fix various issues with RFD and FTLs
cover Fix various issues with RFD and FTLs
Sean Young (4):
mtd: rfd_ftl: allow use of MTD_RAM for testing purposes
mtd: rfd_ftl: add discard support
mtd: blk_devs: make discard work on FTLs
mtd: rfd_ftl: fix use-after-free
drivers/mtd/mtd_blkdevs.c | 1 + drivers/mtd/rfd_ftl.c | 44 ++++++++++++++++++++++++++++++++------- 2 files changed, 38 insertions(+), 7 deletions(-)
Signature · 6 lines
2.31.1 ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/
PATCH 2/4 mtd: rfd_ftl: add discard support
I proposed this change 16 years ago before discard was a feature in the block layer: https://lwn.net/Articles/162776/
Now that the block layer has discard, we can finally merge this change.
--- drivers/mtd/rfd_ftl.c | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+)
drivers/mtd/rfd_ftl.c +29 -0
@@ -705,6 +705,34 @@ static int rfd_ftl_writesect(struct mtd_blktrans_dev *dev, u_long sector, char *return rc; } +static int rfd_ftl_discardsect (struct mtd_blktrans_dev *dev, +unsigned long sector,unsigned int nr_sects) +{ +struct partition *part = (struct partition *)dev; +u_long addr; +int rc; + +while (nr_sects) { +if (sector >= part->sector_count) +return -EIO ; + + addr = part->sector_map[sector]; + +if (addr !=-1 ) { + rc =mark_sector_deleted (part, addr); +if (rc) +return rc; + + part->sector_map[sector] =-1 ; + } + + sector++; + nr_sects--; + } + +return 0 ; +} +static int rfd_ftl_getgeo (struct mtd_blktrans_dev *dev,struct hd_geometry *geo) {struct partition *part = (struct partition *)dev;
@@ -786,6 +814,7 @@ static struct mtd_blktrans_ops rfd_ftl_tr = { .readsect = rfd_ftl_readsect, .writesect = rfd_ftl_writesect, + .discard = rfd_ftl_discardsect, .getgeo = rfd_ftl_getgeo, .add_mtd = rfd_ftl_add_mtd, .remove_dev = rfd_ftl_remove_dev,
Signature · 6 lines
2.31.1 ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/
PATCH 3/4 mtd: blk_devs: make discard work on FTLs
In reply to Sean YoungWithout this change, any discard ioctl fails with -ENOTSUPP and the following logged:
rfda: Error: discard_granularity is 0.
rfda: Error: discard_granularity is 0.
--- drivers/mtd/mtd_blkdevs.c | 1 + 1 file changed, 1 insertion(+)
drivers/mtd/mtd_blkdevs.c +1 -0
@@ -419,6 +419,7 @@ int add_mtd_blktrans_dev(struct mtd_blktrans_dev *new)if (tr->discard) {blk_queue_flag_set (QUEUE_FLAG_DISCARD , new->rq);blk_queue_max_discard_sectors (new->rq,UINT_MAX ); + new->rq->limits.discard_granularity =512 ; } gd->queue = new->rq;
Signature · 6 lines
2.31.1 ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/
PATCH 1/4 mtd: rfd_ftl: allow use of MTD_RAM for testing purposes
In reply to Sean Young--- drivers/mtd/rfd_ftl.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
drivers/mtd/rfd_ftl.c +2 -1
@@ -720,7 +720,8 @@ static void rfd_ftl_add_mtd(struct mtd_blktrans_ops *tr, struct mtd_info *mtd) {struct partition *part; -if (mtd->type !=MTD_NORFLASH || mtd->size >UINT_MAX ) +if ((mtd->type !=MTD_NORFLASH && mtd->type !=MTD_RAM ) || + mtd->size >UINT_MAX )return ; part =kzalloc (sizeof (struct partition ),GFP_KERNEL );
Signature · 6 lines
2.31.1 ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/
PATCH 4/4 mtd: rfd_ftl: fix use-after-free
In reply to Sean Youngdel_mtd_blktrans_dev() will kfree part, so this is a use-after-free. Use container_of() to make it clearer what the cast is doing.
--- drivers/mtd/rfd_ftl.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-)
drivers/mtd/rfd_ftl.c +7 -7
@@ -239,7 +239,7 @@ static int scan_header(struct partition *part)static int rfd_ftl_readsect (struct mtd_blktrans_dev *dev,u_long sector,char *buf) { -struct partition *part = (struct partition *)dev; + struct partition *part = container_of(dev, struct partition, mbd);u_long addr;size_t retlen;int rc;
@@ -600,7 +600,7 @@ static int find_free_sector(const struct partition *part, const struct block *blstatic int do_writesect (struct mtd_blktrans_dev *dev,u_long sector,char *buf,ulong *old_addr) { -struct partition *part = (struct partition *)dev; + struct partition *part = container_of(dev, struct partition, mbd);struct block *block;u_long addr;int i;
@@ -666,7 +666,7 @@ static int do_writesect(struct mtd_blktrans_dev *dev, u_long sector, char *buf,static int rfd_ftl_writesect (struct mtd_blktrans_dev *dev,u_long sector,char *buf) { -struct partition *part = (struct partition *)dev; + struct partition *part = container_of(dev, struct partition, mbd);u_long old_addr;int i;int rc =0 ;
@@ -708,7 +708,7 @@ static int rfd_ftl_writesect(struct mtd_blktrans_dev *dev, u_long sector, char *static int rfd_ftl_discardsect (struct mtd_blktrans_dev *dev,unsigned long sector,unsigned int nr_sects) { -struct partition *part = (struct partition *)dev; + struct partition *part = container_of(dev, struct partition, mbd);u_long addr;int rc;
@@ -735,7 +735,7 @@ static int rfd_ftl_discardsect(struct mtd_blktrans_dev *dev,static int rfd_ftl_getgeo (struct mtd_blktrans_dev *dev,struct hd_geometry *geo) { -struct partition *part = (struct partition *)dev; + struct partition *part = container_of(dev, struct partition, mbd); geo->heads =1 ; geo->sectors =SECTORS_PER_TRACK ;
@@ -792,7 +792,7 @@ static void rfd_ftl_add_mtd(struct mtd_blktrans_ops *tr, struct mtd_info *mtd)static void rfd_ftl_remove_dev (struct mtd_blktrans_dev *dev) { -struct partition *part = (struct partition *)dev; + struct partition *part = container_of(dev, struct partition, mbd);int i;for (i=0 ; i<part->total_blocks; i++) {
@@ -800,10 +800,10 @@ static void rfd_ftl_remove_dev(struct mtd_blktrans_dev *dev) part->mbd.mtd->name, i, part->blocks[i].erases); } -del_mtd_blktrans_dev (dev);vfree (part->sector_map);kfree (part->header_cache);kfree (part->blocks); +del_mtd_blktrans_dev (&part->mbd); }static struct mtd_blktrans_ops rfd_ftl_tr = {
Signature · 6 lines
2.31.1 ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/
Hi Miquel, Richard, Vignesh,
Quoted 9 lines · Sean Young
Sean Young (4):
mtd:rfd_ftl: allow use ofMTD_RAMfor testing purposes
mtd:rfd_ftl: add discard support
mtd:blk_devs: make discard work on FTLs
mtd:rfd_ftl: fix use-after-freedrivers/mtd/mtd_blkdevs.c | 1 + drivers/mtd/rfd_ftl.c | 44 ++++++++++++++++++++++++++++++++------- 2 files changed, 38 insertions(+), 7 deletions(-)
Do the mtd maintainers have any feedback on this patch series, or will they be accepted?
Many thanks,
Sean
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
Miquel Raynal
maintainerRe: [PATCH 1/4] mtd: rfd_ftl: allow use of MTD_RAM for testing purposes
In reply to Sean YoungHi Sean,
Sean Young <[email protected]> wrote on Tue, 13 Jul 2021 10:44:00 +0100:
You miss a commit message here.
Is this a real patch or just something for debugging purpose that we should keep out of tree? I don't really see the reason for this patch even though I am not strongly opposed to it.
Quoted 10 lines
Signed-off-by Sean Young <[email protected]>--- drivers/mtd/rfd_ftl.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)drivers/mtd/rfd_ftl.c +0
-0
drivers/mtd/rfd_ftl.c static void rfd_ftl_add_mtd(struct mtd_blktrans_ops *tr, struct mtd_info *mtd) · line 727
Quoted 9 lines
drivers/mtd/rfd_ftl.c +2
-1@@ -720,7 +720,8 @@ static void rfd_ftl_add_mtd(struct mtd_blktrans_ops *tr, struct mtd_info *mtd) {struct partition *part; -if (mtd->type !=MTD_NORFLASH || mtd->size >UINT_MAX ) +if ((mtd->type !=MTD_NORFLASH && mtd->type !=MTD_RAM ) || + mtd->size >UINT_MAX )return ; part =kzalloc (sizeof (struct partition ),GFP_KERNEL );
Thanks,
Miquèl
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
Miquel Raynal
maintainerRe: [PATCH 2/4] mtd: rfd_ftl: add discard support
In reply to Sean YoungHi Sean,
Sean Young <[email protected]> wrote on Tue, 13 Jul 2021 10:44:01 +0100:
Quoted 4 lines
I proposed this change 16 years ago before discard was a feature in the block layer: https://lwn.net/Articles/162776/
Now that the block layer has discard, we can finally merge this change.
Can you explain why this is needed here?
(also please add the maintainers in Cc: for the v2)
Quoted 45 lines
Signed-off-by Sean Young <[email protected]>--- drivers/mtd/rfd_ftl.c | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+)drivers/mtd/rfd_ftl.c +28
-0@@ -705,6 +705,34 @@ static int rfd_ftl_writesect(struct mtd_blktrans_dev *dev, u_long sector, char *return rc; } +static int rfd_ftl_discardsect (struct mtd_blktrans_dev *dev, +unsigned long sector,unsigned int nr_sects) +{ +struct partition *part = (struct partition *)dev; +u_long addr; +int rc; + +while (nr_sects) { +if (sector >= part->sector_count) +return -EIO ; + + addr = part->sector_map[sector]; + +if (addr !=-1 ) { + rc =mark_sector_deleted (part, addr); +if (rc) +return rc; + + part->sector_map[sector] =-1 ; + } + + sector++; + nr_sects--; + } + +return 0 ; +} +static int rfd_ftl_getgeo (struct mtd_blktrans_dev *dev,struct hd_geometry *geo) {struct partition *part = (struct partition *)dev;
drivers/mtd/rfd_ftl.c static struct mtd_blktrans_ops rfd_ftl_tr = { · line 820
Quoted 7 lines
drivers/mtd/rfd_ftl.c +1
-0@@ -786,6 +814,7 @@ static struct mtd_blktrans_ops rfd_ftl_tr = { .readsect = rfd_ftl_readsect, .writesect = rfd_ftl_writesect, + .discard = rfd_ftl_discardsect, .getgeo = rfd_ftl_getgeo, .add_mtd = rfd_ftl_add_mtd, .remove_dev = rfd_ftl_remove_dev,
Thanks,
Miquèl
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
Miquel Raynal
maintainerRe: [PATCH 4/4] mtd: rfd_ftl: fix use-after-free
In reply to Sean YoungHi Sean,
Sean Young <[email protected]> wrote on Tue, 13 Jul 2021 10:44:03 +0100:
Quoted 12 lines
del_mtd_blktrans_dev()will kfree part, so this is a use-after-free. Usecontainer_of()to make it clearer what the cast is doing.Signed-off-by Sean Young <[email protected]>--- drivers/mtd/rfd_ftl.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-)drivers/mtd/rfd_ftl.c +0
-0
[...]
drivers/mtd/rfd_ftl.c static void rfd_ftl_remove_dev(struct
Quoted 1 line
drivers/mtd/rfd_ftl.c +0
-0@@ -800,10 +800,10 @@ static void rfd_ftl_remove_dev(struct
mtd_blktrans_dev *dev)
drivers/mtd/rfd_ftl.c static void rfd_ftl_remove_dev(struct mtd_blktrans_dev *dev) · line 806
Quoted 8 lines
drivers/mtd/rfd_ftl.c +1
-1part->mbd.mtd->name, i, part->blocks[i].erases); } -del_mtd_blktrans_dev (dev);vfree (part->sector_map);kfree (part->header_cache);kfree (part->blocks); +del_mtd_blktrans_dev (&part->mbd);
I am not sure moving this call at the bottom of ftl_remove_dev makes sense, can we keep it where it was and just do the s/dev/part->mbd/ ?
drivers/mtd/rfd_ftl.c static void rfd_ftl_remove_dev(struct mtd_blktrans_dev *dev) · line 809
Quoted 3 lines
drivers/mtd/rfd_ftl.c +0
-0}static struct mtd_blktrans_ops rfd_ftl_tr = {
Thanks,
Miquèl
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
Re: [PATCH 1/4] mtd: rfd_ftl: allow use of MTD_RAM for testing purposes
In reply to Miquel RaynalQuoted 9 lines · Miquel Raynal
Hi Sean,
Sean Young <[email protected]> wrote on Tue, 13 Jul 2021 10:44:00 +0100:
You miss a commit message here.
Is this a real patch or just something for debugging purpose that we should keep out of tree? I don't really see the reason for this patch even though I am not strongly opposed to it.
This allows the mtdram module to be used as a test bed. This means different mtd sizes can be tested and it can be tested on platforms without an mtd device.
I'll add a better commit message for v2.
Thanks for the review
Sean
Quoted 19 lines
Quoted 19 lines
Signed-off-by Sean Young <[email protected]>--- drivers/mtd/rfd_ftl.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)drivers/mtd/rfd_ftl.c +2
-1@@ -720,7 +720,8 @@ static void rfd_ftl_add_mtd(struct mtd_blktrans_ops *tr, struct mtd_info *mtd) {struct partition *part; -if (mtd->type !=MTD_NORFLASH || mtd->size >UINT_MAX ) +if ((mtd->type !=MTD_NORFLASH && mtd->type !=MTD_RAM ) || + mtd->size >UINT_MAX )return ;
Quoted 8 lines
drivers/mtd/rfd_ftl.c static void rfd_ftl_add_mtd(struct mtd_blktrans_ops *tr, struct mtd_info *mtd) · line 727
Quoted 1 line
drivers/mtd/rfd_ftl.c +0
-0part =kzalloc (sizeof (struct partition ),GFP_KERNEL );Thanks,
Miquèl______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
Re: [PATCH 4/4] mtd: rfd_ftl: fix use-after-free
In reply to Miquel RaynalQuoted 24 lines · Miquel Raynal
Hi Sean,
Sean Young <[email protected]> wrote on Tue, 13 Jul 2021 10:44:03 +0100:
Quoted 12 lines
del_mtd_blktrans_dev()will kfree part, so this is a use-after-free. Usecontainer_of()to make it clearer what the cast is doing.Signed-off-by Sean Young <[email protected]>--- drivers/mtd/rfd_ftl.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-)drivers/mtd/rfd_ftl.c +0
-0[...]
drivers/mtd/rfd_ftl.c static void rfd_ftl_remove_dev(struct
Quoted 1 line
drivers/mtd/rfd_ftl.c +0
-0@@ -800,10 +800,10 @@ static void rfd_ftl_remove_dev(struct
mtd_blktrans_dev*dev)Quoted 3 lines
drivers/mtd/rfd_ftl.c +0
-0part->mbd.mtd->name, i, part->blocks[i].erases); }
Quoted 8 lines
drivers/mtd/rfd_ftl.c static void rfd_ftl_remove_dev(struct mtd_blktrans_dev *dev) · line 806
Quoted 5 lines
drivers/mtd/rfd_ftl.c +1
-1-del_mtd_blktrans_dev (dev);vfree (part->sector_map);kfree (part->header_cache);kfree (part->blocks); +del_mtd_blktrans_dev (&part->mbd);I am not sure moving this call at the bottom of
ftl_remove_devmakes sense, can we keep it where it was and just do the s/dev/part->mbd/ ?
The reason for this patch is that del_mtd_blktrans_dev() kfrees its argument, so both part and dev point to freed memory. This means it's a use after free.
Thanks,
Sean
Quoted 11 lines
drivers/mtd/rfd_ftl.c static void rfd_ftl_remove_dev(struct mtd_blktrans_dev *dev) · line 809
Quoted 4 lines
drivers/mtd/rfd_ftl.c +0
-0}static struct mtd_blktrans_ops rfd_ftl_tr = {Thanks,
Miquèl______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
Re: [PATCH 2/4] mtd: rfd_ftl: add discard support
In reply to Miquel RaynalQuoted 10 lines · Miquel Raynal
Hi Sean,
Sean Young <[email protected]> wrote on Tue, 13 Jul 2021 10:44:01 +0100:
Quoted 4 lines
I proposed this change 16 years ago before discard was a feature in the block layer: https://lwn.net/Articles/162776/
Now that the block layer has discard, we can finally merge this change.
Can you explain why this is needed here?
Because discard is a huge win. This is also known as trim.
By implementing discard, both fstrim and the discard filesystem option can be used.
Implementing discard in the ftl means that when files are removed, there is less data in the ftl mapping. This means less stuff to move around for erasing and also less erasing to do. This means better wear levelling.
There is also a wikipedia page on trim here:
https://en.wikipedia.org/wiki/Trim_(computing)
Thanks,
Sean
Quoted 61 lines
(also please add the maintainers in Cc: for the v2)
Quoted 45 lines
Signed-off-by Sean Young <[email protected]>--- drivers/mtd/rfd_ftl.c | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+)drivers/mtd/rfd_ftl.c +28
-0@@ -705,6 +705,34 @@ static int rfd_ftl_writesect(struct mtd_blktrans_dev *dev, u_long sector, char *return rc; } +static int rfd_ftl_discardsect (struct mtd_blktrans_dev *dev, +unsigned long sector,unsigned int nr_sects) +{ +struct partition *part = (struct partition *)dev; +u_long addr; +int rc; + +while (nr_sects) { +if (sector >= part->sector_count) +return -EIO ; + + addr = part->sector_map[sector]; + +if (addr !=-1 ) { + rc =mark_sector_deleted (part, addr); +if (rc) +return rc; + + part->sector_map[sector] =-1 ; + } + + sector++; + nr_sects--; + } + +return 0 ; +} +static int rfd_ftl_getgeo (struct mtd_blktrans_dev *dev,struct hd_geometry *geo) {struct partition *part = (struct partition *)dev;drivers/mtd/rfd_ftl.c static struct mtd_blktrans_ops rfd_ftl_tr = { · line 820
Quoted 7 lines
drivers/mtd/rfd_ftl.c +1
-0@@ -786,6 +814,7 @@ static struct mtd_blktrans_ops rfd_ftl_tr = { .readsect = rfd_ftl_readsect, .writesect = rfd_ftl_writesect, + .discard = rfd_ftl_discardsect, .getgeo = rfd_ftl_getgeo, .add_mtd = rfd_ftl_add_mtd, .remove_dev = rfd_ftl_remove_dev,Thanks,
Miquèl
Quoted 3 lines
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
Miquel Raynal
maintainerRe: [PATCH 4/4] mtd: rfd_ftl: fix use-after-free
In reply to Sean YoungHi Sean,
Sean Young <[email protected]> wrote on Sat, 7 Aug 2021 08:57:35 +0100:
Quoted 34 lines · Miquel Raynal
Quoted 24 lines
Hi Sean,
Sean Young <[email protected]> wrote on Tue, 13 Jul 2021 10:44:03 +0100:
Quoted 12 lines
del_mtd_blktrans_dev()will kfree part, so this is a use-after-free. Usecontainer_of()to make it clearer what the cast is doing.Signed-off-by Sean Young <[email protected]>--- drivers/mtd/rfd_ftl.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-)drivers/mtd/rfd_ftl.c +0
-0[...]
drivers/mtd/rfd_ftl.c static void rfd_ftl_remove_dev(struct
Quoted 1 line
drivers/mtd/rfd_ftl.c +0
-0@@ -800,10 +800,10 @@ static void rfd_ftl_remove_dev(struct
mtd_blktrans_dev*dev)Quoted 3 lines
drivers/mtd/rfd_ftl.c +0
-0part->mbd.mtd->name, i, part->blocks[i].erases); }Quoted 8 lines
drivers/mtd/rfd_ftl.c static void rfd_ftl_remove_dev(struct mtd_blktrans_dev *dev) · line 806
Quoted 5 lines
drivers/mtd/rfd_ftl.c +1
-1-del_mtd_blktrans_dev (dev);vfree (part->sector_map);kfree (part->header_cache);kfree (part->blocks); +del_mtd_blktrans_dev (&part->mbd);I am not sure moving this call at the bottom of
ftl_remove_devmakes sense, can we keep it where it was and just do the s/dev/part->mbd/ ?
Quoted 2 lines
The reason for this patch is that
del_mtd_blktrans_dev()kfrees its argument, so both part and dev point to freed memory. This means it's a use after free.
Ok, please split this into two patches and we'll be good.
Thanks,
Miquèl
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
Miquel Raynal
maintainerRe: [PATCH 2/4] mtd: rfd_ftl: add discard support
In reply to Sean YoungHi Sean,
Sean Young <[email protected]> wrote on Sat, 7 Aug 2021 09:06:33 +0100:
Quoted 12 lines · Miquel Raynal
Quoted 10 lines
Hi Sean,
Sean Young <[email protected]> wrote on Tue, 13 Jul 2021 10:44:01 +0100:
Quoted 4 lines
I proposed this change 16 years ago before discard was a feature in the block layer: https://lwn.net/Articles/162776/
Now that the block layer has discard, we can finally merge this change.
Can you explain why this is needed here?
Quoted 8 lines
Because discard is a huge win. This is also known as trim.
By implementing discard, both fstrim and the discard filesystem option can be used.
Implementing discard in the ftl means that when files are removed, there is less data in the ftl mapping. This means less stuff to move around for erasing and also less erasing to do. This means better wear levelling.
And this is a good commit message for v2 :-)
Quoted 4 lines
There is also a wikipedia page on trim here:
Got it, thanks!
Quoted 69 lines
Thanks,
Sean
Quoted 64 lines
(also please add the maintainers in Cc: for the v2)
Quoted 45 lines
Signed-off-by Sean Young <[email protected]>--- drivers/mtd/rfd_ftl.c | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+)drivers/mtd/rfd_ftl.c +28
-0@@ -705,6 +705,34 @@ static int rfd_ftl_writesect(struct mtd_blktrans_dev *dev, u_long sector, char *return rc; } +static int rfd_ftl_discardsect (struct mtd_blktrans_dev *dev, +unsigned long sector,unsigned int nr_sects) +{ +struct partition *part = (struct partition *)dev; +u_long addr; +int rc; + +while (nr_sects) { +if (sector >= part->sector_count) +return -EIO ; + + addr = part->sector_map[sector]; + +if (addr !=-1 ) { + rc =mark_sector_deleted (part, addr); +if (rc) +return rc; + + part->sector_map[sector] =-1 ; + } + + sector++; + nr_sects--; + } + +return 0 ; +} +static int rfd_ftl_getgeo (struct mtd_blktrans_dev *dev,struct hd_geometry *geo) {struct partition *part = (struct partition *)dev;drivers/mtd/rfd_ftl.c static struct mtd_blktrans_ops rfd_ftl_tr = { · line 820
Quoted 7 lines
drivers/mtd/rfd_ftl.c +1
-0@@ -786,6 +814,7 @@ static struct mtd_blktrans_ops rfd_ftl_tr = { .readsect = rfd_ftl_readsect, .writesect = rfd_ftl_writesect, + .discard = rfd_ftl_discardsect, .getgeo = rfd_ftl_getgeo, .add_mtd = rfd_ftl_add_mtd, .remove_dev = rfd_ftl_remove_dev,Thanks,
Miquèl______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
Thanks,
Miquèl
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
Re: [PATCH 4/4] mtd: rfd_ftl: fix use-after-free
In reply to Miquel RaynalHi Miquel,
Quoted 38 lines · Miquel Raynal
Hi Sean,
Sean Young <[email protected]> wrote on Sat, 7 Aug 2021 08:57:35 +0100:
Quoted 34 lines · Miquel Raynal
Quoted 32 lines
Hi Sean,
Sean Young <[email protected]> wrote on Tue, 13 Jul 2021 10:44:03 +0100:
Quoted 12 lines
del_mtd_blktrans_dev()will kfree part, so this is a use-after-free. Usecontainer_of()to make it clearer what the cast is doing.Signed-off-by Sean Young <[email protected]>--- drivers/mtd/rfd_ftl.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-)drivers/mtd/rfd_ftl.c +0
-0[...]
drivers/mtd/rfd_ftl.c static void rfd_ftl_remove_dev(struct
Quoted 1 line
drivers/mtd/rfd_ftl.c +0
-0@@ -800,10 +800,10 @@ static void rfd_ftl_remove_dev(struct
mtd_blktrans_dev*dev)drivers/mtd/rfd_ftl.c static void rfd_ftl_remove_dev(struct mtd_blktrans_dev *dev) · line 806
Quoted 8 lines
drivers/mtd/rfd_ftl.c +1
-1part->mbd.mtd->name, i, part->blocks[i].erases); } -del_mtd_blktrans_dev (dev);vfree (part->sector_map);kfree (part->header_cache);kfree (part->blocks); +del_mtd_blktrans_dev (&part->mbd);I am not sure moving this call at the bottom of
ftl_remove_devmakes sense, can we keep it where it was and just do the s/dev/part->mbd/ ?
Quoted 4 lines
Quoted 2 lines
The reason for this patch is that
del_mtd_blktrans_dev()kfrees its argument, so both part and dev point to freed memory. This means it's a use after free.Ok, please split this into two patches and we'll be good.
Good point.
Thank you for the review.
I'll send out v2 shortly.
Sean
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/