Skip to contentSkip to conversation

ata: fixes kernel crash while tracing ata_eh_link_autopsy event

PATCH New Rameshwar Prasad Sahu 2 participants 4 messages Oct 25, 2017 – Nov 2, 2017 1 message not in the archive

Subsystems (3)
SCSI Tracing libata
lore
Share & tools
lore

Rameshwar Prasad Sahu

author
#0 lore raw

PATCH 1/1 ata: fixes kernel crash while tracing ata_eh_link_autopsy event

When tracing ata link error event, the kernel crashes when the disk is removed due to NULL pointer access by trace_ata_eh_link_autopsy API. This occurs as the dev is NULL when the disk disappeared. Given that the trace routine requires only the link info, pass the link info instead of passing dev pointer to fix this kernel crash.

Signed-off-by Rameshwar Prasad Sahu <[email protected]>
---
 drivers/ata/libata-eh.c       |    2 +-
 include/trace/events/libata.h |    9 +++++----
 2 files changed, 6 insertions(+), 5 deletions(-)
drivers/ata/libata-eh.c +1 -1
index e4effef..ab50e7d 100644
@@ -2265,7 +2265,7 @@ static void ata_eh_link_autopsy(struct ata_link *link)
 			eflags |= ATA_EFLAG_DUBIOUS_XFER;
 		ehc->i.action |= ata_eh_speed_down(dev, eflags, all_err_mask);
 	}
-	trace_ata_eh_link_autopsy(dev, ehc->i.action, all_err_mask);
+	trace_ata_eh_link_autopsy(link, ehc->i.action, all_err_mask);
 	DPRINTK("EXIT\n");
 }
include/trace/events/libata.h +5 -4
index 2fbbf99..eff035e 100644
@@ -276,9 +276,10 @@
 
 TRACE_EVENT(ata_eh_link_autopsy,
 
-	TP_PROTO(struct ata_device *dev, unsigned int eh_action, unsigned int eh_err_mask),
+	TP_PROTO(struct ata_link *link, unsigned int eh_action,
+		 unsigned int eh_err_mask),
 
-	TP_ARGS(dev, eh_action, eh_err_mask),
+	TP_ARGS(link, eh_action, eh_err_mask),
 
 	TP_STRUCT__entry(
 		__field( unsigned int,	ata_port )

@@ -288,8 +289,8 @@
 	),
 
 	TP_fast_assign(
-		__entry->ata_port	= dev->link->ap->print_id;
-		__entry->ata_dev	= dev->link->pmp + dev->devno;
+		__entry->ata_port	= link->ap->print_id;
+		__entry->ata_dev	= link->pmp + link->device->devno;
 		__entry->eh_action	= eh_action;
 		__entry->eh_err_mask	= eh_err_mask;
 	),

--
1.7.1

#1 lore raw

PATCH 1/1 ata: fixes kernel crash while tracing ata_eh_link_autopsy event

Hello,

include/trace/events/libata.h line 293

Quoted 8 lines · Rameshwar Prasad Sahu
include/trace/events/libata.h +2 -2
@@ -288,8 +289,8 @@
 	),
 
 	TP_fast_assign(
-		__entry->ata_port	= dev->link->ap->print_id;
-		__entry->ata_dev	= dev->link->pmp + dev->devno;
+		__entry->ata_port	= link->ap->print_id;
+		__entry->ata_dev	= link->pmp + link->device->devno;

The above is wrong if there are multiple devices on the link. It probably should take both link and dev and use dev iff it's not NULL.

Thanks.

Signature · 1 line
tejun

A message that is not in the archive (sent off-list or not yet received): CAFd313w9W2hHFPJ47y0jeNb1u7=0deLtwcRF=+HXGBYft-QaLw@mail.gmail.com

#3 lore raw

PATCH 1/1 ata: fixes kernel crash while tracing ata_eh_link_autopsy event

Hello,

Quoted 5 lines · Rameshwar Sahu
Quoted 2 lines

probably should take both link and dev and use dev iff it's not NULL.

Instead of this would it be better to call trace_ata_eh_link_autopsy() if dev is not NULL from ata error handler ??

Oh yeah, that'd work too and be probably better.

Thanks.

Signature · 1 line
tejun
author
#4 lore raw

PATCH 1/1 ata: fixes kernel crash while tracing ata_eh_link_autopsy event

Quoted 10 lines · Tejun Heo

Hello,

Quoted 5 lines · Rameshwar Sahu
Quoted 2 lines

probably should take both link and dev and use dev iff it's not NULL.

Instead of this would it be better to call trace_ata_eh_link_autopsy() if dev is not NULL from ata error handler ??

Oh yeah, that'd work too and be probably better.

I will post another version with this fix.

Quoted 5 lines

Thanks.

--
tejun