CDL related fixes
Damien Le Moal
maintainercover CDL related fixes
A couple of fixes for CDL support. The first patch addresses a potential NULL pointer dereference in ata_msense_control_spgt2() with drives not supporting CDL. The second patch fixes ata_msense_control() to report the correct T2A and T2B sub pages when CDL is supported.
Damien Le Moal (2):
ata: libata-scsi: Fix ata_msense_control_spgt2()
ata: libata-scsi: Fix ata_msense_control() CDL page reporting
drivers/ata/libata-scsi.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-)
Signature · 1 line
2.46.0
Damien Le Moal
maintainerPATCH 1/2 ata: libata-scsi: Fix ata_msense_control_spgt2()
ata_msense_control_spgt2() can be called even for devices that do not support CDL when the user requests the ALL_SUB_MPAGES mode sense page, but for such device, this will cause a NULL pointer dereference as dev->cdl is NULL. Similarly, we should not return any data if ata_msense_control_spgt2() is called when the user requested the CDL_T2A_SUB_MPAGE or CDL_T2B_SUB_MPAGE pages for a device that does not support CDL.
Avoid this potential NULL pointer dereference by checking if the device support CDL on entry to ata_msense_control_spgt2() and return 0 if it does not support CDL.
--- drivers/ata/libata-scsi.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)
drivers/ata/libata-scsi.c +6 -1
@@ -2245,10 +2245,15 @@ static inline u16 ata_xlat_cdl_limit(u8 *buf)static unsigned int ata_msense_control_spgt2 (struct ata_device *dev,u8 *buf,u8 spg) { -u8 *b, *cdl = dev->cdl->desc_log_buf, *desc; +u8 *b, *cdl, *desc;u32 policy;int i; +if (!(dev->flags &ATA_DFLAG_CDL ) || !dev->cdl) +return 0 ; + + cdl = dev->cdl->desc_log_buf; +/* * Fill the subpage. The first four bytes of the T2A/T2B mode pages * are a header. The PAGE LENGTH field is the size of the page
Signature · 1 line
2.46.0
Re: [PATCH 1/2] ata: libata-scsi: Fix ata_msense_control_spgt2()
Quoted 31 lines · Damien Le Moal
ata_msense_control_spgt2()can be called even for devices that do not support CDL when the user requests theALL_SUB_MPAGESmode sense page, but for such device, this will cause a NULL pointer dereference asdev->cdlis NULL. Similarly, we should not return any data ifata_msense_control_spgt2()is called when the user requested theCDL_T2A_SUB_MPAGEorCDL_T2B_SUB_MPAGEpages for a device that does not support CDL.Avoid this potential NULL pointer dereference by checking if the device support CDL on entry to
ata_msense_control_spgt2()and return 0 if it does not support CDL.Reported-by [email protected]Tested-by [email protected]Fixes 602bcf212637 ("ata: libata: Improve CDL resource management")Signed-off-by Damien Le Moal <[email protected]>--- drivers/ata/libata-scsi.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)drivers/ata/libata-scsi.c +1
-1@@ -2245,10 +2245,15 @@ static inline u16 ata_xlat_cdl_limit(u8 *buf)static unsigned int ata_msense_control_spgt2 (struct ata_device *dev,u8 *buf,u8 spg) { -u8 *b, *cdl = dev->cdl->desc_log_buf, *desc; +u8 *b, *cdl, *desc;u32 policy;
drivers/ata/libata-scsi.c static inline u16 ata_xlat_cdl_limit(u8 *buf) · line 2259
Quoted 10 lines
drivers/ata/libata-scsi.c +5
-0int i; +if (!(dev->flags &ATA_DFLAG_CDL ) || !dev->cdl) +return 0 ; + + cdl = dev->cdl->desc_log_buf; +/* * Fill the subpage. The first four bytes of the T2A/T2B mode pages * are a header. The PAGE LENGTH field is the size of the page
Cheers,
Hannes
Signature · 4 lines
Dr. Hannes Reinecke Kernel Storage Architect [email protected] +49 911 74053 688 SUSE Software Solutions GmbH, Frankenstr. 146, 90461 Nürnberg HRB 36809 (AG Nürnberg), GF: I. Totev, A. McDonald, W. Knoblich
Niklas Cassel
maintainerRe: [PATCH 1/2] ata: libata-scsi: Fix ata_msense_control_spgt2()
In reply to Damien Le MoalQuoted 8 lines · Damien Le Moal
ata_msense_control_spgt2()can be called even for devices that do not support CDL when the user requests theALL_SUB_MPAGESmode sense page, but for such device, this will cause a NULL pointer dereference asdev->cdlis NULL. Similarly, we should not return any data ifata_msense_control_spgt2()is called when the user requested theCDL_T2A_SUB_MPAGEorCDL_T2B_SUB_MPAGEpages for a device that does not support CDL.
Quoted 7 lines
Avoid this potential NULL pointer dereference by checking if the device support CDL on entry to
ata_msense_control_spgt2()and return 0 if it does not support CDL.Reported-by [email protected]Tested-by [email protected]Fixes 602bcf212637 ("ata: libata: Improve CDL resource management")
Looks good, but the commit above also changes ata_eh_get_ncq_success_sense(), and also adds calls to free the resources in ata_dev_free_resources(), which is called by different EH paths, so perhaps we also need a:
if (!dev->cdl)
guard in ata_eh_get_ncq_success_sense() ?
EH is a bit complex, but it would make sense if EH already ensures that ata_eh_get_ncq_success_sense() can't be called after ata_dev_free_resources() has been called. In that case, we shouldn't need to add an additional guard.
Kind regards,
Niklas
Damien Le Moal
maintainerPATCH 2/2 ata: libata-scsi: Fix ata_msense_control() CDL page reporting
In reply to Damien Le MoalWhen the user requests the ALL_SUB_MPAGES mode sense page, ata_msense_control() adds the CDL_T2A_SUB_MPAGE twice instead of adding the CDL_T2A_SUB_MPAGE and CDL_T2B_SUB_MPAGE pages information. Correct the second call to ata_msense_control_spgt2() to report the CDL_T2B_SUB_MPAGE page.
--- drivers/ata/libata-scsi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/ata/libata-scsi.c +1 -1
@@ -2350,7 +2350,7 @@ static unsigned int ata_msense_control(struct ata_device *dev, u8 *buf,case ALL_SUB_MPAGES : n =ata_msense_control_spg0 (dev, buf, changeable); n +=ata_msense_control_spgt2 (dev, buf + n,CDL_T2A_SUB_MPAGE ); - n +=ata_msense_control_spgt2 (dev, buf + n,CDL_T2A_SUB_MPAGE ); + n +=ata_msense_control_spgt2 (dev, buf + n,CDL_T2B_SUB_MPAGE ); n +=ata_msense_control_ata_feature (dev, buf + n);return n;default :
Signature · 1 line
2.46.0
Re: [PATCH 2/2] ata: libata-scsi: Fix ata_msense_control() CDL page reporting
Quoted 18 lines · Damien Le Moal
When the user requests the
ALL_SUB_MPAGESmode sense page,ata_msense_control()adds theCDL_T2A_SUB_MPAGEtwice instead of adding theCDL_T2A_SUB_MPAGEandCDL_T2B_SUB_MPAGEpages information. Correct the second call toata_msense_control_spgt2()to report theCDL_T2B_SUB_MPAGEpage.Fixes 673b2fe6ff1d ("scsi: ata: libata-scsi: Add support for CDL pages mode sense")Signed-off-by Damien Le Moal <[email protected]>--- drivers/ata/libata-scsi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)drivers/ata/libata-scsi.c +0
-0
drivers/ata/libata-scsi.c static unsigned int ata_msense_control(struct ata_device *dev, u8 *buf, · line 2356
Quoted 8 lines
drivers/ata/libata-scsi.c +1
-1@@ -2350,7 +2350,7 @@ static unsigned int ata_msense_control(struct ata_device *dev, u8 *buf,case ALL_SUB_MPAGES : n =ata_msense_control_spg0 (dev, buf, changeable); n +=ata_msense_control_spgt2 (dev, buf + n,CDL_T2A_SUB_MPAGE ); - n +=ata_msense_control_spgt2 (dev, buf + n,CDL_T2A_SUB_MPAGE ); + n +=ata_msense_control_spgt2 (dev, buf + n,CDL_T2B_SUB_MPAGE ); n +=ata_msense_control_ata_feature (dev, buf + n);return n;default :
Cheers,
Hannes
Signature · 4 lines
Dr. Hannes Reinecke Kernel Storage Architect [email protected] +49 911 74053 688 SUSE Software Solutions GmbH, Frankenstr. 146, 90461 Nürnberg HRB 36809 (AG Nürnberg), GF: I. Totev, A. McDonald, W. Knoblich
Quoted 5 lines · Damien Le Moal
A couple of fixes for CDL support. The first patch addresses a potential NULL pointer dereference in
ata_msense_control_spgt2()with drives not supporting CDL. The second patch fixesata_msense_control()to report the correct T2A and T2B sub pages when CDL is supported.
Quoted 10 lines
Damien Le Moal (2):
ata: libata-scsi: Fixata_msense_control_spgt2()
ata: libata-scsi: Fixata_msense_control()CDL page reportingdrivers/ata/libata-scsi.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-)Signature · 1 line
2.46.0
For the series: