All of lore.kernel.org
 help / color / mirror / Atom feed
* [patch] [SCSI] atp870u: 64 bit bug in probe()
@ 2013-09-04  9:50 ` Dan Carpenter
  0 siblings, 0 replies; 27+ messages in thread
From: Dan Carpenter @ 2013-09-04  9:50 UTC (permalink / raw)
  To: James E.J. Bottomley; +Cc: linux-scsi, kernel-janitors

On 64 bit CPUs there is a memory corruption bug on probe().  It should
be setting 32 bits of data on both 32 and 64 bit.

Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
Static checker stuff.  I can't test this.

diff --git a/drivers/scsi/atp870u.c b/drivers/scsi/atp870u.c
index 15a629d..3edbf30 100644
--- a/drivers/scsi/atp870u.c
+++ b/drivers/scsi/atp870u.c
@@ -2791,11 +2791,11 @@ next_fblk_885:
 		    p->global_map[m]= 0;
 		    for (k=0; k < 4; k++) {
 			outw(n++,base_io + 0x3c);
-			((unsigned long *)&setupdata[m][0])[k]=inl(base_io + 0x38);
+			((u32 *)&setupdata[m][0])[k]=inl(base_io + 0x38);
 		    }
 		    for (k=0; k < 4; k++) {
 			outw(n++,base_io + 0x3c);
-			((unsigned long *)&p->sp[m][0])[k]=inl(base_io + 0x38);
+			((u32 *)&p->sp[m][0])[k]=inl(base_io + 0x38);
 		    }
 		    n += 8;
 		}

^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [patch] [SCSI] atp870u: 64 bit bug in probe()
@ 2013-09-04  9:50 ` Dan Carpenter
  0 siblings, 0 replies; 27+ messages in thread
From: Dan Carpenter @ 2013-09-04  9:50 UTC (permalink / raw)
  To: James E.J. Bottomley; +Cc: linux-scsi, kernel-janitors

On 64 bit CPUs there is a memory corruption bug on probe().  It should
be setting 32 bits of data on both 32 and 64 bit.

Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
Static checker stuff.  I can't test this.

diff --git a/drivers/scsi/atp870u.c b/drivers/scsi/atp870u.c
index 15a629d..3edbf30 100644
--- a/drivers/scsi/atp870u.c
+++ b/drivers/scsi/atp870u.c
@@ -2791,11 +2791,11 @@ next_fblk_885:
 		    p->global_map[m]= 0;
 		    for (k=0; k < 4; k++) {
 			outw(n++,base_io + 0x3c);
-			((unsigned long *)&setupdata[m][0])[k]=inl(base_io + 0x38);
+			((u32 *)&setupdata[m][0])[k]=inl(base_io + 0x38);
 		    }
 		    for (k=0; k < 4; k++) {
 			outw(n++,base_io + 0x3c);
-			((unsigned long *)&p->sp[m][0])[k]=inl(base_io + 0x38);
+			((u32 *)&p->sp[m][0])[k]=inl(base_io + 0x38);
 		    }
 		    n += 8;
 		}

^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [patch] [SCSI] atp870u: 64 bit bug in probe()
  2013-09-04  9:50 ` Dan Carpenter
@ 2015-07-29 21:36   ` Dan Carpenter
  -1 siblings, 0 replies; 27+ messages in thread
From: Dan Carpenter @ 2015-07-29 21:36 UTC (permalink / raw)
  To: James E.J. Bottomley; +Cc: linux-scsi, kernel-janitors

On 64 bit CPUs there is a memory corruption bug on probe().  It should
be a u32 pointer instead of an unsigned long pointer or we write past
the end of the setupdata[] array.

Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
Someone reported in 2003 that probe has a NULL deref so maybe it's
related to this memory corruption?
https://bugzilla.kernel.org/show_bug.cgi?id\x1118

If only we had applied this patch when I originally sent it two years
ago, then it would only be 10 years too late instead of 12!  :P

diff --git a/drivers/scsi/atp870u.c b/drivers/scsi/atp870u.c
index 05301bc..62acabd 100644
--- a/drivers/scsi/atp870u.c
+++ b/drivers/scsi/atp870u.c
@@ -2791,11 +2791,11 @@ next_fblk_885:
 		    p->global_map[m]= 0;
 		    for (k=0; k < 4; k++) {
 			outw(n++,base_io + 0x3c);
-			((unsigned long *)&setupdata[m][0])[k]=inl(base_io + 0x38);
+			((u32 *)&setupdata[m][0])[k]=inl(base_io + 0x38);
 		    }
 		    for (k=0; k < 4; k++) {
 			outw(n++,base_io + 0x3c);
-			((unsigned long *)&p->sp[m][0])[k]=inl(base_io + 0x38);
+			((u32 *)&p->sp[m][0])[k]=inl(base_io + 0x38);
 		    }
 		    n += 8;
 		}

^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [patch] [SCSI] atp870u: 64 bit bug in probe()
@ 2015-07-29 21:36   ` Dan Carpenter
  0 siblings, 0 replies; 27+ messages in thread
From: Dan Carpenter @ 2015-07-29 21:36 UTC (permalink / raw)
  To: James E.J. Bottomley; +Cc: linux-scsi, kernel-janitors

On 64 bit CPUs there is a memory corruption bug on probe().  It should
be a u32 pointer instead of an unsigned long pointer or we write past
the end of the setupdata[] array.

Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
Someone reported in 2003 that probe has a NULL deref so maybe it's
related to this memory corruption?
https://bugzilla.kernel.org/show_bug.cgi?id=1118

If only we had applied this patch when I originally sent it two years
ago, then it would only be 10 years too late instead of 12!  :P

diff --git a/drivers/scsi/atp870u.c b/drivers/scsi/atp870u.c
index 05301bc..62acabd 100644
--- a/drivers/scsi/atp870u.c
+++ b/drivers/scsi/atp870u.c
@@ -2791,11 +2791,11 @@ next_fblk_885:
 		    p->global_map[m]= 0;
 		    for (k=0; k < 4; k++) {
 			outw(n++,base_io + 0x3c);
-			((unsigned long *)&setupdata[m][0])[k]=inl(base_io + 0x38);
+			((u32 *)&setupdata[m][0])[k]=inl(base_io + 0x38);
 		    }
 		    for (k=0; k < 4; k++) {
 			outw(n++,base_io + 0x3c);
-			((unsigned long *)&p->sp[m][0])[k]=inl(base_io + 0x38);
+			((u32 *)&p->sp[m][0])[k]=inl(base_io + 0x38);
 		    }
 		    n += 8;
 		}

^ permalink raw reply related	[flat|nested] 27+ messages in thread

* Re: [patch] [SCSI] atp870u: 64 bit bug in probe()
  2015-07-29 21:36   ` Dan Carpenter
@ 2015-07-30  6:54     ` Hannes Reinecke
  -1 siblings, 0 replies; 27+ messages in thread
From: Hannes Reinecke @ 2015-07-30  6:54 UTC (permalink / raw)
  To: Dan Carpenter, James E.J. Bottomley; +Cc: linux-scsi, kernel-janitors

On 07/29/2015 11:36 PM, Dan Carpenter wrote:
> On 64 bit CPUs there is a memory corruption bug on probe().  It should
> be a u32 pointer instead of an unsigned long pointer or we write past
> the end of the setupdata[] array.
> 
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> ---
> Someone reported in 2003 that probe has a NULL deref so maybe it's
> related to this memory corruption?
> https://bugzilla.kernel.org/show_bug.cgi?id\x1118
> 
> If only we had applied this patch when I originally sent it two years
> ago, then it would only be 10 years too late instead of 12!  :P
> 
Reviewed-by: Hannes Reinecke <hare@suse.com>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		               zSeries & Storage
hare@suse.de			               +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)
--
To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [patch] [SCSI] atp870u: 64 bit bug in probe()
@ 2015-07-30  6:54     ` Hannes Reinecke
  0 siblings, 0 replies; 27+ messages in thread
From: Hannes Reinecke @ 2015-07-30  6:54 UTC (permalink / raw)
  To: Dan Carpenter, James E.J. Bottomley; +Cc: linux-scsi, kernel-janitors

On 07/29/2015 11:36 PM, Dan Carpenter wrote:
> On 64 bit CPUs there is a memory corruption bug on probe().  It should
> be a u32 pointer instead of an unsigned long pointer or we write past
> the end of the setupdata[] array.
> 
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> ---
> Someone reported in 2003 that probe has a NULL deref so maybe it's
> related to this memory corruption?
> https://bugzilla.kernel.org/show_bug.cgi?id=1118
> 
> If only we had applied this patch when I originally sent it two years
> ago, then it would only be 10 years too late instead of 12!  :P
> 
Reviewed-by: Hannes Reinecke <hare@suse.com>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		               zSeries & Storage
hare@suse.de			               +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 27+ messages in thread

* [patch RESEND] atp870u: 64 bit bug in atp885_init()
  2015-07-30  6:54     ` Hannes Reinecke
@ 2015-12-09 10:24       ` Dan Carpenter
  -1 siblings, 0 replies; 27+ messages in thread
From: Dan Carpenter @ 2015-12-09 10:24 UTC (permalink / raw)
  To: James E.J. Bottomley, Ondrej Zary
  Cc: Martin K. Petersen, linux-scsi, linux-kernel, kernel-janitors,
	Hannes Reinecke

On 64 bit CPUs there is a memory corruption bug on probe().  It should
be a u32 pointer instead of an unsigned long pointer or we write past
the end of the setupdata[] array.

Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
Reviewed-by: Hannes Reinecke <hare@suse.com>
---
Resending because we have shuffled the code around so the patch needed
to be refreshed against linux-next.  Although I do wonder why we are
still working on this code since it has never worked on 64 bit systems
so probably all the users gave up a decade ago.

diff --git a/drivers/scsi/atp870u.c b/drivers/scsi/atp870u.c
index 8b52a9d..b46997c 100644
--- a/drivers/scsi/atp870u.c
+++ b/drivers/scsi/atp870u.c
@@ -1413,11 +1413,11 @@ static void atp885_init(struct Scsi_Host *shpnt)
 			atpdev->global_map[m] = 0;
 			for (k = 0; k < 4; k++) {
 				atp_writew_base(atpdev, 0x3c, n++);
-				((unsigned long *)&setupdata[m][0])[k] = atp_readl_base(atpdev, 0x38);
+				((u32 *)&setupdata[m][0])[k] = atp_readl_base(atpdev, 0x38);
 			}
 			for (k = 0; k < 4; k++) {
 				atp_writew_base(atpdev, 0x3c, n++);
-				((unsigned long *)&atpdev->sp[m][0])[k] = atp_readl_base(atpdev, 0x38);
+				((u32 *)&atpdev->sp[m][0])[k] = atp_readl_base(atpdev, 0x38);
 			}
 			n += 8;
 		}

^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [patch RESEND] atp870u: 64 bit bug in atp885_init()
@ 2015-12-09 10:24       ` Dan Carpenter
  0 siblings, 0 replies; 27+ messages in thread
From: Dan Carpenter @ 2015-12-09 10:24 UTC (permalink / raw)
  To: James E.J. Bottomley, Ondrej Zary
  Cc: Martin K. Petersen, linux-scsi, linux-kernel, kernel-janitors,
	Hannes Reinecke

On 64 bit CPUs there is a memory corruption bug on probe().  It should
be a u32 pointer instead of an unsigned long pointer or we write past
the end of the setupdata[] array.

Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
Reviewed-by: Hannes Reinecke <hare@suse.com>
---
Resending because we have shuffled the code around so the patch needed
to be refreshed against linux-next.  Although I do wonder why we are
still working on this code since it has never worked on 64 bit systems
so probably all the users gave up a decade ago.

diff --git a/drivers/scsi/atp870u.c b/drivers/scsi/atp870u.c
index 8b52a9d..b46997c 100644
--- a/drivers/scsi/atp870u.c
+++ b/drivers/scsi/atp870u.c
@@ -1413,11 +1413,11 @@ static void atp885_init(struct Scsi_Host *shpnt)
 			atpdev->global_map[m] = 0;
 			for (k = 0; k < 4; k++) {
 				atp_writew_base(atpdev, 0x3c, n++);
-				((unsigned long *)&setupdata[m][0])[k] = atp_readl_base(atpdev, 0x38);
+				((u32 *)&setupdata[m][0])[k] = atp_readl_base(atpdev, 0x38);
 			}
 			for (k = 0; k < 4; k++) {
 				atp_writew_base(atpdev, 0x3c, n++);
-				((unsigned long *)&atpdev->sp[m][0])[k] = atp_readl_base(atpdev, 0x38);
+				((u32 *)&atpdev->sp[m][0])[k] = atp_readl_base(atpdev, 0x38);
 			}
 			n += 8;
 		}

^ permalink raw reply related	[flat|nested] 27+ messages in thread

* Re: [patch RESEND] atp870u: 64 bit bug in atp885_init()
  2015-12-09 10:24       ` Dan Carpenter
@ 2015-12-09 11:53         ` One Thousand Gnomes
  -1 siblings, 0 replies; 27+ messages in thread
From: One Thousand Gnomes @ 2015-12-09 11:53 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: James E.J. Bottomley, Ondrej Zary, Martin K. Petersen,
	linux-scsi, linux-kernel, kernel-janitors, Hannes Reinecke

On Wed, 9 Dec 2015 13:24:53 +0300
Dan Carpenter <dan.carpenter@oracle.com> wrote:

> On 64 bit CPUs there is a memory corruption bug on probe().  It should
> be a u32 pointer instead of an unsigned long pointer or we write past
> the end of the setupdata[] array.
> 
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> Reviewed-by: Hannes Reinecke <hare@suse.com>
> ---
> Resending because we have shuffled the code around so the patch needed
> to be refreshed against linux-next.  Although I do wonder why we are
> still working on this code since it has never worked on 64 bit systems
> so probably all the users gave up a decade ago.

So this is untested ? If so please make it very clear in the commit
message because the kernel is IMHO getting too full of polished, neat,
recently modified, never tested, never used code.

I agree it would be better if the driver was simply deleted. I've not
even seen an ATP870 bug report in years.

Alan

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [patch RESEND] atp870u: 64 bit bug in atp885_init()
@ 2015-12-09 11:53         ` One Thousand Gnomes
  0 siblings, 0 replies; 27+ messages in thread
From: One Thousand Gnomes @ 2015-12-09 11:53 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: James E.J. Bottomley, Ondrej Zary, Martin K. Petersen,
	linux-scsi, linux-kernel, kernel-janitors, Hannes Reinecke

On Wed, 9 Dec 2015 13:24:53 +0300
Dan Carpenter <dan.carpenter@oracle.com> wrote:

> On 64 bit CPUs there is a memory corruption bug on probe().  It should
> be a u32 pointer instead of an unsigned long pointer or we write past
> the end of the setupdata[] array.
> 
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> Reviewed-by: Hannes Reinecke <hare@suse.com>
> ---
> Resending because we have shuffled the code around so the patch needed
> to be refreshed against linux-next.  Although I do wonder why we are
> still working on this code since it has never worked on 64 bit systems
> so probably all the users gave up a decade ago.

So this is untested ? If so please make it very clear in the commit
message because the kernel is IMHO getting too full of polished, neat,
recently modified, never tested, never used code.

I agree it would be better if the driver was simply deleted. I've not
even seen an ATP870 bug report in years.

Alan

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [patch RESEND] atp870u: 64 bit bug in atp885_init()
  2015-12-09 11:53         ` One Thousand Gnomes
@ 2015-12-09 12:07           ` Ondrej Zary
  -1 siblings, 0 replies; 27+ messages in thread
From: Ondrej Zary @ 2015-12-09 12:07 UTC (permalink / raw)
  To: One Thousand Gnomes
  Cc: Dan Carpenter, James E.J. Bottomley, Martin K. Petersen,
	linux-scsi, linux-kernel, kernel-janitors, Hannes Reinecke

On Wednesday 09 December 2015 12:53:39 One Thousand Gnomes wrote:
> On Wed, 9 Dec 2015 13:24:53 +0300
> Dan Carpenter <dan.carpenter@oracle.com> wrote:
> 
> > On 64 bit CPUs there is a memory corruption bug on probe().  It should
> > be a u32 pointer instead of an unsigned long pointer or we write past
> > the end of the setupdata[] array.
> > 
> > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> > Reviewed-by: Hannes Reinecke <hare@suse.com>
> > ---
> > Resending because we have shuffled the code around so the patch needed
> > to be refreshed against linux-next.  Although I do wonder why we are
> > still working on this code since it has never worked on 64 bit systems
> > so probably all the users gave up a decade ago.
> 
> So this is untested ? If so please make it very clear in the commit
> message because the kernel is IMHO getting too full of polished, neat,
> recently modified, never tested, never used code.
> 
> I agree it would be better if the driver was simply deleted. I've not
> even seen an ATP870 bug report in years.

Maybe because it worked. Although the code was horrible. I've done some big changes to this driver recently (tested, of course).
I can't test this patch as I don't have ATP885 card, only ATP870.

-- 
Ondrej Zary

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [patch RESEND] atp870u: 64 bit bug in atp885_init()
@ 2015-12-09 12:07           ` Ondrej Zary
  0 siblings, 0 replies; 27+ messages in thread
From: Ondrej Zary @ 2015-12-09 12:07 UTC (permalink / raw)
  To: One Thousand Gnomes
  Cc: Dan Carpenter, James E.J. Bottomley, Martin K. Petersen,
	linux-scsi, linux-kernel, kernel-janitors, Hannes Reinecke

On Wednesday 09 December 2015 12:53:39 One Thousand Gnomes wrote:
> On Wed, 9 Dec 2015 13:24:53 +0300
> Dan Carpenter <dan.carpenter@oracle.com> wrote:
> 
> > On 64 bit CPUs there is a memory corruption bug on probe().  It should
> > be a u32 pointer instead of an unsigned long pointer or we write past
> > the end of the setupdata[] array.
> > 
> > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> > Reviewed-by: Hannes Reinecke <hare@suse.com>
> > ---
> > Resending because we have shuffled the code around so the patch needed
> > to be refreshed against linux-next.  Although I do wonder why we are
> > still working on this code since it has never worked on 64 bit systems
> > so probably all the users gave up a decade ago.
> 
> So this is untested ? If so please make it very clear in the commit
> message because the kernel is IMHO getting too full of polished, neat,
> recently modified, never tested, never used code.
> 
> I agree it would be better if the driver was simply deleted. I've not
> even seen an ATP870 bug report in years.

Maybe because it worked. Although the code was horrible. I've done some big changes to this driver recently (tested, of course).
I can't test this patch as I don't have ATP885 card, only ATP870.

-- 
Ondrej Zary

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [patch RESEND] atp870u: 64 bit bug in atp885_init()
  2015-12-09 11:53         ` One Thousand Gnomes
@ 2015-12-09 13:45           ` Dan Carpenter
  -1 siblings, 0 replies; 27+ messages in thread
From: Dan Carpenter @ 2015-12-09 13:45 UTC (permalink / raw)
  To: One Thousand Gnomes
  Cc: James E.J. Bottomley, Ondrej Zary, Martin K. Petersen,
	linux-scsi, linux-kernel, kernel-janitors, Hannes Reinecke

Everyone knows I didn't test it but it's an obvious one line fix for
memory corruption.  If no one uses the code, at least this is harmless
and silences a static checker warning.

In olden times we used to say, "Oh this bounds checking is crap but it's
root only so let's leave it alone."  But these days we just fix it.
It's easier to just fix everything instead of trying to decide which
bugs are critical.

regards,
dan carpenter


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [patch RESEND] atp870u: 64 bit bug in atp885_init()
@ 2015-12-09 13:45           ` Dan Carpenter
  0 siblings, 0 replies; 27+ messages in thread
From: Dan Carpenter @ 2015-12-09 13:45 UTC (permalink / raw)
  To: One Thousand Gnomes
  Cc: James E.J. Bottomley, Ondrej Zary, Martin K. Petersen,
	linux-scsi, linux-kernel, kernel-janitors, Hannes Reinecke

Everyone knows I didn't test it but it's an obvious one line fix for
memory corruption.  If no one uses the code, at least this is harmless
and silences a static checker warning.

In olden times we used to say, "Oh this bounds checking is crap but it's
root only so let's leave it alone."  But these days we just fix it.
It's easier to just fix everything instead of trying to decide which
bugs are critical.

regards,
dan carpenter


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [patch RESEND] atp870u: 64 bit bug in atp885_init()
  2015-12-09 13:45           ` Dan Carpenter
@ 2015-12-09 14:14             ` One Thousand Gnomes
  -1 siblings, 0 replies; 27+ messages in thread
From: One Thousand Gnomes @ 2015-12-09 14:14 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: James E.J. Bottomley, Ondrej Zary, Martin K. Petersen,
	linux-scsi, linux-kernel, kernel-janitors, Hannes Reinecke

On Wed, 9 Dec 2015 16:45:12 +0300
Dan Carpenter <dan.carpenter@oracle.com> wrote:

> Everyone knows I didn't test it but it's an obvious one line fix for
> memory corruption.  If no one uses the code, at least this is harmless
> and silences a static checker warning.
> 
> In olden times we used to say, "Oh this bounds checking is crap but it's
> root only so let's leave it alone."  But these days we just fix it.
> It's easier to just fix everything instead of trying to decide which
> bugs are critical.

Unfortunately it's all too easy to look down 50 commit messages to an
apaprently active file all "fixing small bugs" or "correcting indenting"
without realising that every single one of them should have been tagged

"[UNTESTED]: "

so that anyone looking at the code can see immediately its historical
hazardous waste.

Alan

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [patch RESEND] atp870u: 64 bit bug in atp885_init()
@ 2015-12-09 14:14             ` One Thousand Gnomes
  0 siblings, 0 replies; 27+ messages in thread
From: One Thousand Gnomes @ 2015-12-09 14:14 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: James E.J. Bottomley, Ondrej Zary, Martin K. Petersen,
	linux-scsi, linux-kernel, kernel-janitors, Hannes Reinecke

On Wed, 9 Dec 2015 16:45:12 +0300
Dan Carpenter <dan.carpenter@oracle.com> wrote:

> Everyone knows I didn't test it but it's an obvious one line fix for
> memory corruption.  If no one uses the code, at least this is harmless
> and silences a static checker warning.
> 
> In olden times we used to say, "Oh this bounds checking is crap but it's
> root only so let's leave it alone."  But these days we just fix it.
> It's easier to just fix everything instead of trying to decide which
> bugs are critical.

Unfortunately it's all too easy to look down 50 commit messages to an
apaprently active file all "fixing small bugs" or "correcting indenting"
without realising that every single one of them should have been tagged

"[UNTESTED]: "

so that anyone looking at the code can see immediately its historical
hazardous waste.

Alan

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [patch RESEND] atp870u: 64 bit bug in atp885_init()
  2015-12-09 14:14             ` One Thousand Gnomes
@ 2015-12-09 17:48               ` Dan Carpenter
  -1 siblings, 0 replies; 27+ messages in thread
From: Dan Carpenter @ 2015-12-09 17:48 UTC (permalink / raw)
  To: One Thousand Gnomes
  Cc: James E.J. Bottomley, Ondrej Zary, Martin K. Petersen,
	linux-scsi, linux-kernel, kernel-janitors, Hannes Reinecke

We should add a tag to indicate that we are sending a patch for a crappy
driver.

IMHO-this-driver-is-garbage: Your Name <email>

If it got 10 votes of no confidence it would be moved to staging and
then deleted.

Anyway, realistically, let's just apply this fix.  It's tempting to
think we could delete all atp885 related code, but maybe people are
still using it with 32 bit kernels.  Or someone could delete it, but I'm
not brave enough to be the one to do it.

regards,
dan carpenter

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [patch RESEND] atp870u: 64 bit bug in atp885_init()
@ 2015-12-09 17:48               ` Dan Carpenter
  0 siblings, 0 replies; 27+ messages in thread
From: Dan Carpenter @ 2015-12-09 17:48 UTC (permalink / raw)
  To: One Thousand Gnomes
  Cc: James E.J. Bottomley, Ondrej Zary, Martin K. Petersen,
	linux-scsi, linux-kernel, kernel-janitors, Hannes Reinecke

We should add a tag to indicate that we are sending a patch for a crappy
driver.

IMHO-this-driver-is-garbage: Your Name <email>

If it got 10 votes of no confidence it would be moved to staging and
then deleted.

Anyway, realistically, let's just apply this fix.  It's tempting to
think we could delete all atp885 related code, but maybe people are
still using it with 32 bit kernels.  Or someone could delete it, but I'm
not brave enough to be the one to do it.

regards,
dan carpenter

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [patch RESEND] atp870u: 64 bit bug in atp885_init()
  2015-12-09 17:48               ` Dan Carpenter
@ 2015-12-09 18:11                 ` Julia Lawall
  -1 siblings, 0 replies; 27+ messages in thread
From: Julia Lawall @ 2015-12-09 18:11 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: One Thousand Gnomes, James E.J. Bottomley, Ondrej Zary,
	Martin K. Petersen, linux-scsi, linux-kernel, kernel-janitors,
	Hannes Reinecke



On Wed, 9 Dec 2015, Dan Carpenter wrote:

> We should add a tag to indicate that we are sending a patch for a crappy
> driver.
>
> IMHO-this-driver-is-garbage: Your Name <email>
>
> If it got 10 votes of no confidence it would be moved to staging and
> then deleted.

Forgive my ignorance, but what is the exact procedure?  For example, the
following file: drivers/pcmcia/vrc4173_cardu.c contains the following
code: INIT_WORK(&socket->tq_work, cardu_bh, socket);.  The last time
INIT_WORK took three arguments was Linux 2.6.19, so I think no one has
been compiling this code recently.  There would be the .c file and the
associated .h file to move to staging, but it's less clear to me eg what
to do with the Kconfig entry and the Makefile entry.  And is there
anything else to take into account?

thanks,
julia

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [patch RESEND] atp870u: 64 bit bug in atp885_init()
@ 2015-12-09 18:11                 ` Julia Lawall
  0 siblings, 0 replies; 27+ messages in thread
From: Julia Lawall @ 2015-12-09 18:11 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: One Thousand Gnomes, James E.J. Bottomley, Ondrej Zary,
	Martin K. Petersen, linux-scsi, linux-kernel, kernel-janitors,
	Hannes Reinecke



On Wed, 9 Dec 2015, Dan Carpenter wrote:

> We should add a tag to indicate that we are sending a patch for a crappy
> driver.
>
> IMHO-this-driver-is-garbage: Your Name <email>
>
> If it got 10 votes of no confidence it would be moved to staging and
> then deleted.

Forgive my ignorance, but what is the exact procedure?  For example, the
following file: drivers/pcmcia/vrc4173_cardu.c contains the following
code: INIT_WORK(&socket->tq_work, cardu_bh, socket);.  The last time
INIT_WORK took three arguments was Linux 2.6.19, so I think no one has
been compiling this code recently.  There would be the .c file and the
associated .h file to move to staging, but it's less clear to me eg what
to do with the Kconfig entry and the Makefile entry.  And is there
anything else to take into account?

thanks,
julia

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [patch RESEND] atp870u: 64 bit bug in atp885_init()
  2015-12-09 18:11                 ` Julia Lawall
@ 2015-12-09 18:28                   ` Dan Carpenter
  -1 siblings, 0 replies; 27+ messages in thread
From: Dan Carpenter @ 2015-12-09 18:28 UTC (permalink / raw)
  To: Julia Lawall
  Cc: One Thousand Gnomes, James E.J. Bottomley, Ondrej Zary,
	Martin K. Petersen, linux-scsi, linux-kernel, kernel-janitors,
	Hannes Reinecke

On Wed, Dec 09, 2015 at 07:11:15PM +0100, Julia Lawall wrote:
> Forgive my ignorance, but what is the exact procedure?  For example, the
> following file: drivers/pcmcia/vrc4173_cardu.c contains the following
> code: INIT_WORK(&socket->tq_work, cardu_bh, socket);.  The last time
> INIT_WORK took three arguments was Linux 2.6.19, so I think no one has
> been compiling this code recently.  There would be the .c file and the
> associated .h file to move to staging, but it's less clear to me eg what
> to do with the Kconfig entry and the Makefile entry.  And is there
> anything else to take into account?

You should just delete that code along with the Kconfig and Makefile
entries.  Don't bother moving it to staging.  The move to staging is
supposed to give people one last chance to yell if they absolutely need
the code.  No one has compiled this code for years so no one will miss
it.

regards,
dan carpenter


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [patch RESEND] atp870u: 64 bit bug in atp885_init()
@ 2015-12-09 18:28                   ` Dan Carpenter
  0 siblings, 0 replies; 27+ messages in thread
From: Dan Carpenter @ 2015-12-09 18:28 UTC (permalink / raw)
  To: Julia Lawall
  Cc: One Thousand Gnomes, James E.J. Bottomley, Ondrej Zary,
	Martin K. Petersen, linux-scsi, linux-kernel, kernel-janitors,
	Hannes Reinecke

On Wed, Dec 09, 2015 at 07:11:15PM +0100, Julia Lawall wrote:
> Forgive my ignorance, but what is the exact procedure?  For example, the
> following file: drivers/pcmcia/vrc4173_cardu.c contains the following
> code: INIT_WORK(&socket->tq_work, cardu_bh, socket);.  The last time
> INIT_WORK took three arguments was Linux 2.6.19, so I think no one has
> been compiling this code recently.  There would be the .c file and the
> associated .h file to move to staging, but it's less clear to me eg what
> to do with the Kconfig entry and the Makefile entry.  And is there
> anything else to take into account?

You should just delete that code along with the Kconfig and Makefile
entries.  Don't bother moving it to staging.  The move to staging is
supposed to give people one last chance to yell if they absolutely need
the code.  No one has compiled this code for years so no one will miss
it.

regards,
dan carpenter


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [patch RESEND] atp870u: 64 bit bug in atp885_init()
  2015-12-09 18:28                   ` Dan Carpenter
@ 2015-12-09 19:37                     ` One Thousand Gnomes
  -1 siblings, 0 replies; 27+ messages in thread
From: One Thousand Gnomes @ 2015-12-09 19:37 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: Julia Lawall, James E.J. Bottomley, Ondrej Zary,
	Martin K. Petersen, linux-scsi, linux-kernel, kernel-janitors,
	Hannes Reinecke

> You should just delete that code along with the Kconfig and Makefile
> entries.  Don't bother moving it to staging.  The move to staging is
> supposed to give people one last chance to yell if they absolutely need
> the code.  No one has compiled this code for years so no one will miss
> it.

And for stuff which might be worth saving (eg something that looks rather
broken but has possibly got users) the driver goes into staging
in its own directory and the Makefile and Kconfig entry for it move into
the staging directory with the hope that someone screams and maintains it.

Alan


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [patch RESEND] atp870u: 64 bit bug in atp885_init()
@ 2015-12-09 19:37                     ` One Thousand Gnomes
  0 siblings, 0 replies; 27+ messages in thread
From: One Thousand Gnomes @ 2015-12-09 19:37 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: Julia Lawall, James E.J. Bottomley, Ondrej Zary,
	Martin K. Petersen, linux-scsi, linux-kernel, kernel-janitors,
	Hannes Reinecke

> You should just delete that code along with the Kconfig and Makefile
> entries.  Don't bother moving it to staging.  The move to staging is
> supposed to give people one last chance to yell if they absolutely need
> the code.  No one has compiled this code for years so no one will miss
> it.

And for stuff which might be worth saving (eg something that looks rather
broken but has possibly got users) the driver goes into staging
in its own directory and the Makefile and Kconfig entry for it move into
the staging directory with the hope that someone screams and maintains it.

Alan


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [patch RESEND] atp870u: 64 bit bug in atp885_init()
  2015-12-09 10:24       ` Dan Carpenter
@ 2018-02-15 23:44         ` Martin K. Petersen
  -1 siblings, 0 replies; 27+ messages in thread
From: Martin K. Petersen @ 2018-02-15 23:44 UTC (permalink / raw)
  To: Ondrej Zary
  Cc: Dan Carpenter, James E.J. Bottomley, Martin K. Petersen,
	linux-scsi, kernel-janitors, Hannes Reinecke


> On 64 bit CPUs there is a memory corruption bug on probe().  It should
> be a u32 pointer instead of an unsigned long pointer or we write past
> the end of the setupdata[] array.

Ondrej, please review!

-- 
Martin K. Petersen	Oracle Linux Engineering

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [patch RESEND] atp870u: 64 bit bug in atp885_init()
@ 2018-02-15 23:44         ` Martin K. Petersen
  0 siblings, 0 replies; 27+ messages in thread
From: Martin K. Petersen @ 2018-02-15 23:44 UTC (permalink / raw)
  To: Ondrej Zary
  Cc: Dan Carpenter, James E.J. Bottomley, Martin K. Petersen,
	linux-scsi, kernel-janitors, Hannes Reinecke


> On 64 bit CPUs there is a memory corruption bug on probe().  It should
> be a u32 pointer instead of an unsigned long pointer or we write past
> the end of the setupdata[] array.

Ondrej, please review!

-- 
Martin K. Petersen	Oracle Linux Engineering

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [patch RESEND] atp870u: 64 bit bug in atp885_init()
  2015-12-09 10:24       ` Dan Carpenter
                         ` (2 preceding siblings ...)
  (?)
@ 2018-03-02  2:11       ` Martin K. Petersen
  -1 siblings, 0 replies; 27+ messages in thread
From: Martin K. Petersen @ 2018-03-02  2:11 UTC (permalink / raw)
  To: kernel-janitors


Dan,

> On 64 bit CPUs there is a memory corruption bug on probe().  It should
> be a u32 pointer instead of an unsigned long pointer or we write past
> the end of the setupdata[] array.

Applied to 4.17/scsi-queue. Thanks!

-- 
Martin K. Petersen	Oracle Linux Engineering

^ permalink raw reply	[flat|nested] 27+ messages in thread

end of thread, other threads:[~2018-03-02  2:11 UTC | newest]

Thread overview: 27+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-09-04  9:50 [patch] [SCSI] atp870u: 64 bit bug in probe() Dan Carpenter
2013-09-04  9:50 ` Dan Carpenter
2015-07-29 21:36 ` Dan Carpenter
2015-07-29 21:36   ` Dan Carpenter
2015-07-30  6:54   ` Hannes Reinecke
2015-07-30  6:54     ` Hannes Reinecke
2015-12-09 10:24     ` [patch RESEND] atp870u: 64 bit bug in atp885_init() Dan Carpenter
2015-12-09 10:24       ` Dan Carpenter
2015-12-09 11:53       ` One Thousand Gnomes
2015-12-09 11:53         ` One Thousand Gnomes
2015-12-09 12:07         ` Ondrej Zary
2015-12-09 12:07           ` Ondrej Zary
2015-12-09 13:45         ` Dan Carpenter
2015-12-09 13:45           ` Dan Carpenter
2015-12-09 14:14           ` One Thousand Gnomes
2015-12-09 14:14             ` One Thousand Gnomes
2015-12-09 17:48             ` Dan Carpenter
2015-12-09 17:48               ` Dan Carpenter
2015-12-09 18:11               ` Julia Lawall
2015-12-09 18:11                 ` Julia Lawall
2015-12-09 18:28                 ` Dan Carpenter
2015-12-09 18:28                   ` Dan Carpenter
2015-12-09 19:37                   ` One Thousand Gnomes
2015-12-09 19:37                     ` One Thousand Gnomes
2018-02-15 23:44       ` Martin K. Petersen
2018-02-15 23:44         ` Martin K. Petersen
2018-03-02  2:11       ` Martin K. Petersen

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.