Re: [PATCH 0/3] ide: Fix use of paired device
On Thursday 18 October 2007, Benjamin Herrenschmidt wrote: > At least 2 drivers (siimage and cs5535) have a bug where they use > the construct: > > ide_drive_t *pair = >drives[drive->dn ^ 1]; > > To access the other drive in a master/slave pair. This is bogus > because drive->dn is not the unit number, but the global drive > number, thus can be 2 & 3 for ide1, 4 & 5 for ide2 etc... > > This causes the driver to access beyond the drive array into lalaland > for any other interface than ide0 and in some case, actually crash :-) > > These 3 patches fix those by introducing a ide_get_paired_drive() > helper that does the right thing and then using it. > > Please apply to 2.6.24 if no objection. Thanks for debugging and fixing this! All three patches applied. Bart - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 0/3] ide: Fix use of paired device
On Thu, 2007-10-18 at 15:54 +0400, Sergei Shtylyov wrote:
> Hello.
>
> Benjamin Herrenschmidt wrote:
>
> > At least 2 drivers (siimage and cs5535) have a bug where they use
> > the construct:
>
> > ide_drive_t *pair = >drives[drive->dn ^ 1];
>
> > To access the other drive in a master/slave pair. This is bogus
> > because drive->dn is not the unit number, but the global drive
> > number, thus can be 2 & 3 for ide1, 4 & 5 for ide2 etc...
>
> Huh? With drive->dn calculated as:
>
> for (unit = 0; unit < MAX_DRIVES; ++unit) {
> ide_drive_t *drive = >drives[unit];
> drive->dn = (hwif->channel ? 2 : 0) + unit;
>
> (with MAX_DRIVES always being 2) how comes it may be 4 or 5?!
>
> > Please apply to 2.6.24 if no objection.
>
> I object. :-)
allright, I should have read the code more closely :-)
I saw drive->dn being 2 on ide1, so the code misbehaving because of
that, and thus drawn an incorrect conclusion.
Still.. the code is bogus and having an helper to get the paired device
makes sense, so all we need to do is fix the implementation of that
helper. The good thing is that I think my patch is still correct, though
the comment is not :-)
Ben.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 0/3] ide: Fix use of paired device
Hello, I wrote:
At least 2 drivers (siimage and cs5535) have a bug where they use
the construct:
ide_drive_t *pair= >drives[drive->dn ^ 1];
To access the other drive in a master/slave pair. This is bogus
because drive->dn is not the unit number, but the global drive
number, thus can be 2 & 3 for ide1, 4 & 5 for ide2 etc...
Huh? With drive->dn calculated as:
for (unit = 0; unit < MAX_DRIVES; ++unit) {
ide_drive_t *drive = >drives[unit];
drive->dn = (hwif->channel ? 2 : 0) + unit;
(with MAX_DRIVES always being 2) how comes it may be 4 or 5?!
Please apply to 2.6.24 if no objection.
I object. :-)
Well, actually no objections about the patch itself since it deals with
the cases of drive->dn being 2 and 3. But this probably be better to be done
all in one patch, or at least in 2 patches (please also update driver version
in the heading comment).
MBR, Sergei
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 0/3] ide: Fix use of paired device
Hello.
Benjamin Herrenschmidt wrote:
At least 2 drivers (siimage and cs5535) have a bug where they use
the construct:
ide_drive_t *pair = >drives[drive->dn ^ 1];
To access the other drive in a master/slave pair. This is bogus
because drive->dn is not the unit number, but the global drive
number, thus can be 2 & 3 for ide1, 4 & 5 for ide2 etc...
Huh? With drive->dn calculated as:
for (unit = 0; unit < MAX_DRIVES; ++unit) {
ide_drive_t *drive = >drives[unit];
drive->dn = (hwif->channel ? 2 : 0) + unit;
(with MAX_DRIVES always being 2) how comes it may be 4 or 5?!
Please apply to 2.6.24 if no objection.
I object. :-)
MBR, Sergei
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 0/3] ide: Fix use of paired device
Hello.
Benjamin Herrenschmidt wrote:
At least 2 drivers (siimage and cs5535) have a bug where they use
the construct:
ide_drive_t *pair = hwif-drives[drive-dn ^ 1];
To access the other drive in a master/slave pair. This is bogus
because drive-dn is not the unit number, but the global drive
number, thus can be 2 3 for ide1, 4 5 for ide2 etc...
Huh? With drive-dn calculated as:
for (unit = 0; unit MAX_DRIVES; ++unit) {
ide_drive_t *drive = hwif-drives[unit];
drive-dn = (hwif-channel ? 2 : 0) + unit;
(with MAX_DRIVES always being 2) how comes it may be 4 or 5?!
Please apply to 2.6.24 if no objection.
I object. :-)
MBR, Sergei
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 0/3] ide: Fix use of paired device
Hello, I wrote:
At least 2 drivers (siimage and cs5535) have a bug where they use
the construct:
ide_drive_t *pair= hwif-drives[drive-dn ^ 1];
To access the other drive in a master/slave pair. This is bogus
because drive-dn is not the unit number, but the global drive
number, thus can be 2 3 for ide1, 4 5 for ide2 etc...
Huh? With drive-dn calculated as:
for (unit = 0; unit MAX_DRIVES; ++unit) {
ide_drive_t *drive = hwif-drives[unit];
drive-dn = (hwif-channel ? 2 : 0) + unit;
(with MAX_DRIVES always being 2) how comes it may be 4 or 5?!
Please apply to 2.6.24 if no objection.
I object. :-)
Well, actually no objections about the patch itself since it deals with
the cases of drive-dn being 2 and 3. But this probably be better to be done
all in one patch, or at least in 2 patches (please also update driver version
in the heading comment).
MBR, Sergei
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 0/3] ide: Fix use of paired device
On Thu, 2007-10-18 at 15:54 +0400, Sergei Shtylyov wrote:
Hello.
Benjamin Herrenschmidt wrote:
At least 2 drivers (siimage and cs5535) have a bug where they use
the construct:
ide_drive_t *pair = hwif-drives[drive-dn ^ 1];
To access the other drive in a master/slave pair. This is bogus
because drive-dn is not the unit number, but the global drive
number, thus can be 2 3 for ide1, 4 5 for ide2 etc...
Huh? With drive-dn calculated as:
for (unit = 0; unit MAX_DRIVES; ++unit) {
ide_drive_t *drive = hwif-drives[unit];
drive-dn = (hwif-channel ? 2 : 0) + unit;
(with MAX_DRIVES always being 2) how comes it may be 4 or 5?!
Please apply to 2.6.24 if no objection.
I object. :-)
allright, I should have read the code more closely :-)
I saw drive-dn being 2 on ide1, so the code misbehaving because of
that, and thus drawn an incorrect conclusion.
Still.. the code is bogus and having an helper to get the paired device
makes sense, so all we need to do is fix the implementation of that
helper. The good thing is that I think my patch is still correct, though
the comment is not :-)
Ben.
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 0/3] ide: Fix use of paired device
On Thursday 18 October 2007, Benjamin Herrenschmidt wrote: At least 2 drivers (siimage and cs5535) have a bug where they use the construct: ide_drive_t *pair = hwif-drives[drive-dn ^ 1]; To access the other drive in a master/slave pair. This is bogus because drive-dn is not the unit number, but the global drive number, thus can be 2 3 for ide1, 4 5 for ide2 etc... This causes the driver to access beyond the drive array into lalaland for any other interface than ide0 and in some case, actually crash :-) These 3 patches fix those by introducing a ide_get_paired_drive() helper that does the right thing and then using it. Please apply to 2.6.24 if no objection. Thanks for debugging and fixing this! All three patches applied. Bart - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 0/3] ide: Fix use of paired device
At least 2 drivers (siimage and cs5535) have a bug where they use the construct: ide_drive_t *pair = >drives[drive->dn ^ 1]; To access the other drive in a master/slave pair. This is bogus because drive->dn is not the unit number, but the global drive number, thus can be 2 & 3 for ide1, 4 & 5 for ide2 etc... This causes the driver to access beyond the drive array into lalaland for any other interface than ide0 and in some case, actually crash :-) These 3 patches fix those by introducing a ide_get_paired_drive() helper that does the right thing and then using it. Please apply to 2.6.24 if no objection. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 0/3] ide: Fix use of paired device
At least 2 drivers (siimage and cs5535) have a bug where they use the construct: ide_drive_t *pair = hwif-drives[drive-dn ^ 1]; To access the other drive in a master/slave pair. This is bogus because drive-dn is not the unit number, but the global drive number, thus can be 2 3 for ide1, 4 5 for ide2 etc... This causes the driver to access beyond the drive array into lalaland for any other interface than ide0 and in some case, actually crash :-) These 3 patches fix those by introducing a ide_get_paired_drive() helper that does the right thing and then using it. Please apply to 2.6.24 if no objection. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
