diff mbox series

ata: fixes kernel crash while tracing ata_eh_link_autopsy event

Message ID 1508926976-11467-1-git-send-email-rsahu@apm.com
State Not Applicable
Delegated to: David Miller
Headers show
Series ata: fixes kernel crash while tracing ata_eh_link_autopsy event | expand

Commit Message

Rameshwar Prasad Sahu Oct. 25, 2017, 10:22 a.m. UTC
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(-)

--
1.7.1

--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Tejun Heo Oct. 25, 2017, 2:54 p.m. UTC | #1
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 Heo Nov. 1, 2017, 2:30 p.m. UTC | #2
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.
Rameshwar Prasad Sahu Nov. 2, 2017, 9:57 a.m. UTC | #3
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
--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox series

Patch

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;
 	),