[PATCH] ata: fixes kernel crash while tracing ata_eh_link_autopsy event

[PATCH] ata: fixes kernel crash while tracing ata_eh_link_autopsy event

[Rameshwar Prasad Sahu]

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 <rsahu@apm.com>
---
 drivers/ata/libata-eh.c       |    2 +-
 include/trace/events/libata.h |    9 +++++----
 2 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/ata/libata-eh.c b/drivers/ata/libata-eh.c
index e4effef..ab50e7d 100644
--- a/drivers/ata/libata-eh.c
+++ b/drivers/ata/libata-eh.c
@@ -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");
 }

diff --git a/include/trace/events/libata.h b/include/trace/events/libata.h
index 2fbbf99..eff035e 100644
--- a/include/trace/events/libata.h
+++ b/include/trace/events/libata.h
@@ -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

[PATCH] ata: fixes kernel crash while tracing ata_eh_link_autopsy event

[Tejun Heo]

Hello,

On Wed, Oct 25, 2017 at 03:52:56PM +0530, Rameshwar Prasad Sahu wrote:
> @@ -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.

-- 
tejun

[PATCH] ata: fixes kernel crash while tracing ata_eh_link_autopsy event

[Tejun Heo]

Hello,

On Tue, Oct 31, 2017 at 08:52:44PM +0530, Rameshwar Sahu wrote:
> > 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.

-- 
tejun

[PATCH] ata: fixes kernel crash while tracing ata_eh_link_autopsy event

[Rameshwar Sahu]

On Wed, Nov 1, 2017 at 8:00 PM, Tejun Heo <tj@kernel.org> wrote:
> Hello,
>
> On Tue, Oct 31, 2017 at 08:52:44PM +0530, Rameshwar Sahu wrote:
>> > 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.
>
> Thanks.
>
> --
> tejun