* [PATCH] ide: fix revision comparison in ide_in_drive_list
@ 2006-06-20 10:52 Kirill Smelkov
2006-06-20 22:19 ` Andrew Morton
0 siblings, 1 reply; 6+ messages in thread
From: Kirill Smelkov @ 2006-06-20 10:52 UTC (permalink / raw)
To: Andrew Morton, B.Zolnierkiewicz; +Cc: linux-kernel, linux-ide
NB: bart/ide-2.6.git seems to be unmaintained, so I'm sending this directly to -mm
Fix ide_in_drive_list: drive_table->id_firmware should be searched *in* id->fw_rev,
not vise versa.
Signed-off-by: Kirill Smelkov <kirr@mns.spb.ru>
Index: linux-2.6.17/drivers/ide/ide-dma.c
===================================================================
--- linux-2.6.17.orig/drivers/ide/ide-dma.c 2006-06-20 13:51:49.000000000 +0400
+++ linux-2.6.17/drivers/ide/ide-dma.c 2006-06-20 13:52:14.000000000 +0400
@@ -147,7 +147,7 @@
{
for ( ; drive_table->id_model ; drive_table++)
if ((!strcmp(drive_table->id_model, id->model)) &&
- ((strstr(drive_table->id_firmware, id->fw_rev)) ||
+ ((strstr(id->fw_rev, drive_table->id_firmware)) ||
(!strcmp(drive_table->id_firmware, "ALL"))))
return 1;
return 0;
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] ide: fix revision comparison in ide_in_drive_list
2006-06-20 10:52 [PATCH] ide: fix revision comparison in ide_in_drive_list Kirill Smelkov
@ 2006-06-20 22:19 ` Andrew Morton
2006-06-20 23:12 ` Alan Cox
0 siblings, 1 reply; 6+ messages in thread
From: Andrew Morton @ 2006-06-20 22:19 UTC (permalink / raw)
To: Kirill Smelkov; +Cc: B.Zolnierkiewicz, linux-kernel, linux-ide, Alan Cox
Kirill Smelkov <kirr@mns.spb.ru> wrote:
>
> NB: bart/ide-2.6.git seems to be unmaintained, so I'm sending this directly to -mm
>
> Fix ide_in_drive_list: drive_table->id_firmware should be searched *in* id->fw_rev,
> not vise versa.
>
> Signed-off-by: Kirill Smelkov <kirr@mns.spb.ru>
>
> Index: linux-2.6.17/drivers/ide/ide-dma.c
> ===================================================================
> --- linux-2.6.17.orig/drivers/ide/ide-dma.c 2006-06-20 13:51:49.000000000 +0400
> +++ linux-2.6.17/drivers/ide/ide-dma.c 2006-06-20 13:52:14.000000000 +0400
> @@ -147,7 +147,7 @@
> {
> for ( ; drive_table->id_model ; drive_table++)
> if ((!strcmp(drive_table->id_model, id->model)) &&
> - ((strstr(drive_table->id_firmware, id->fw_rev)) ||
> + ((strstr(id->fw_rev, drive_table->id_firmware)) ||
> (!strcmp(drive_table->id_firmware, "ALL"))))
> return 1;
> return 0;
hm. This seems... rather serious. I assume that in most cases, the
firmware rev which we have in the table (eg "24.09P07") is a full-string
match for the string which the drive returned.
If not, you've just blacklisted a whole bunch of drives which always should
have been blacklisted, but weren't.
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] ide: fix revision comparison in ide_in_drive_list
2006-06-20 22:19 ` Andrew Morton
@ 2006-06-20 23:12 ` Alan Cox
2006-06-21 6:17 ` Kirill Smelkov
0 siblings, 1 reply; 6+ messages in thread
From: Alan Cox @ 2006-06-20 23:12 UTC (permalink / raw)
To: Andrew Morton; +Cc: Kirill Smelkov, B.Zolnierkiewicz, linux-kernel, linux-ide
Ar Maw, 2006-06-20 am 15:19 -0700, ysgrifennodd Andrew Morton:
> hm. This seems... rather serious. I assume that in most cases, the
> firmware rev which we have in the table (eg "24.09P07") is a full-string
> match for the string which the drive returned.
They are full matches as far as I can see so it should be ok, plus the
DMA blacklist is mostly hardware that went out ten or more years ago.
Even if it did mis-trigger we'd be blacklisting extra firmware revs of
iffy hardware so that isn't do worrying.
Alan
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] ide: fix revision comparison in ide_in_drive_list
2006-06-20 23:12 ` Alan Cox
@ 2006-06-21 6:17 ` Kirill Smelkov
2006-06-21 11:44 ` Alan Cox
0 siblings, 1 reply; 6+ messages in thread
From: Kirill Smelkov @ 2006-06-21 6:17 UTC (permalink / raw)
To: Alan Cox; +Cc: Andrew Morton, B.Zolnierkiewicz, linux-kernel, linux-ide
21 Jun 2006 03:12, Alan Cox wrote:
> Ar Maw, 2006-06-20 am 15:19 -0700, ysgrifennodd Andrew Morton:
> > hm. This seems... rather serious. I assume that in most cases, the
> > firmware rev which we have in the table (eg "24.09P07") is a full-string
> > match for the string which the drive returned.
>
> They are full matches as far as I can see so it should be ok, plus the
> DMA blacklist is mostly hardware that went out ten or more years ago.
> Even if it did mis-trigger we'd be blacklisting extra firmware revs of
> iffy hardware so that isn't do worrying.
No, they are *not* full matches.
My case:
CF model="TRANSCEND", revision="20050811" as reported by "hdparm -I" and bios,
but
id->model="TRANSCEND",
id->fw_rev="20050811TRANSCEND"
note the trailing in id->fw_rev,
So lets look at ide_in_drive_list -- it should try to detect blacklisted revision,
so it looks for substr device_table->id_firmware in id->fw_rev.
NB: I don't know what causes id->fw_rev to be bogus (?), but maybe the right fix
NB: is to correct its initialization. Some person with IDE background should clarify this.
--
Kirill
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] ide: fix revision comparison in ide_in_drive_list
2006-06-21 6:17 ` Kirill Smelkov
@ 2006-06-21 11:44 ` Alan Cox
2006-06-21 13:11 ` Kirill Smelkov
0 siblings, 1 reply; 6+ messages in thread
From: Alan Cox @ 2006-06-21 11:44 UTC (permalink / raw)
To: Kirill Smelkov; +Cc: Andrew Morton, B.Zolnierkiewicz, linux-kernel, linux-ide
Ar Mer, 2006-06-21 am 10:17 +0400, ysgrifennodd Kirill Smelkov:
> id->model="TRANSCEND",
> id->fw_rev="20050811TRANSCEND"
>
> note the trailing in id->fw_rev,
These are not null terminated strings in the ident block. So if you
merely print them or test against them you'll break on 8 char long
firmware names. They may also be space rather than \0 padded if shorter
Alan
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] ide: fix revision comparison in ide_in_drive_list
2006-06-21 11:44 ` Alan Cox
@ 2006-06-21 13:11 ` Kirill Smelkov
0 siblings, 0 replies; 6+ messages in thread
From: Kirill Smelkov @ 2006-06-21 13:11 UTC (permalink / raw)
To: Alan Cox; +Cc: Andrew Morton, B.Zolnierkiewicz, linux-kernel, linux-ide
21 Jun 2006 15:44, Alan Cox wrote:
> > id->model="TRANSCEND",
> > id->fw_rev="20050811TRANSCEND"
> >
> > note the trailing in id->fw_rev,
>
> These are not null terminated strings in the ident block. So if you
> merely print them or test against them you'll break on 8 char long
> firmware names. They may also be space rather than \0 padded if shorter
This justifies strstr usage.
Thanks again.
--
Kirill
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2006-06-21 13:08 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2006-06-20 10:52 [PATCH] ide: fix revision comparison in ide_in_drive_list Kirill Smelkov
2006-06-20 22:19 ` Andrew Morton
2006-06-20 23:12 ` Alan Cox
2006-06-21 6:17 ` Kirill Smelkov
2006-06-21 11:44 ` Alan Cox
2006-06-21 13:11 ` Kirill Smelkov
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox