Skip to contentSkip to conversation

fix kernel oops with CF-Cards

PATCH In review Thomas Kleffel 2 participants 2 messages Sep 6, 2005 – Sep 12, 2005 libata

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 fix kernel oops with CF-Cards

Hello,

when a mounted CF-Card is removed from the system, inserted back into the slot, removed again, and then umount is called for that device the kernel oopes.

(This is a slightly different issue than noted in my last mail.)

This happens because the reference counting gets confused. When a disk gets released by ide_disk_release() it sets the driver_data member of the corresponding drive to NULL. This is bad, as the pyhsical drive could be assigned to another idkp structure in the meantime (happens, when the drive is removed and inserted again).

My fix is to simply leave the drive alone when a disk is released. This shouldn't cause any side-effects - drive->driver_data isn't tested for containing NULL anywhere.

The following patch (against vanilla 2.6.13) fixes that problem:

drivers/ide/ide-disk.c +0 -3
diff -uprN -X b/Documentation/dontdiff a/drivers/ide/ide-disk.c b/drivers/ide/ide-disk.c
@@ -1048,11 +1048,8 @@ static int ide_disk_remove(struct device
 static void ide_disk_release(struct kref *kref)
 {
 	struct ide_disk_obj *idkp = to_ide_disk(kref);
-	ide_drive_t *drive = idkp->drive;
 	struct gendisk *g = idkp->disk;
 
-	drive->driver_data = NULL;
-	drive->devfs_name[0] = '\0';
 	g->private_data = NULL;
 	put_disk(g);
 	kfree(idkp);

Best regards,
Thomas

#1 lore raw
Quoted 1 line · Thomas Kleffel

Hello,

Hi,

Quoted 11 lines

when a mounted CF-Card is removed from the system, inserted back into the slot, removed again, and then umount is called for that device the kernel oopes.

(This is a slightly different issue than noted in my last mail.)

This happens because the reference counting gets confused. When a disk gets released by ide_disk_release() it sets the driver_data member of the corresponding drive to NULL. This is bad, as the pyhsical drive could be assigned to another idkp structure in the meantime (happens, when the drive is removed and inserted again).

As another idkp structure is a new object so still keeping the reference to the old one is a bug. It looks like the real problem here is that there are still references to the old idkp object while it is already gone.

Please see my previous mail.

Thanks,
Bartlomiej

Quoted 27 lines

My fix is to simply leave the drive alone when a disk is released. This shouldn't cause any side-effects - drive->driver_data isn't tested for containing NULL anywhere.

The following patch (against vanilla 2.6.13) fixes that problem:

diff -uprN -X b/Documentation/dontdiff a/drivers/ide/ide-disk.c b/drivers/ide/ide-disk.c

drivers/ide/ide-disk.c 2005-09-05 02:10:30.000000000 +0200 renamed from drivers/ide/ide-disk.c 2005-08-24 17:58:02.000000000 +0200 +0 -3
@@ -1048,11 +1048,8 @@ static int ide_disk_remove(struct device
  static void ide_disk_release(struct kref *kref)
  {
        struct ide_disk_obj *idkp = to_ide_disk(kref);
-       ide_drive_t *drive = idkp->drive;
        struct gendisk *g = idkp->disk;
 
-       drive->driver_data = NULL;
-       drive->devfs_name[0] = '\0';
        g->private_data = NULL;
        put_disk(g);
        kfree(idkp);

Best regards,
Thomas