potential memory overrun in ata_id_to_hd_driveid() on big endian machines

Please have a look at the usage of ata_id_to_hd_driveid() in ide-ioctls.c.

u16 array 'id' is allocated depending on the command, which might result in 142 byte. Indexing into the array at position 'ATA_ID_LBA_CAPACITY_2' in ata_id_to_hd_driveid() would overrun the allocated memory in that case.

Regards,
Christian

ide-ioctls.c:

	static int ide_get_identity_ioctl(ide_drive_t *drive, unsigned int cmd,
					  unsigned long arg)

	u16 *id = NULL;
	int size = (cmd == HDIO_GET_IDENTITY) ? (ATA_ID_WORDS * 2) : 142;

	..

	id = kmalloc(size, GFP_KERNEL);

	..

	ata_id_to_hd_driveid(id);

	..

ata.h:

	static inline void ata_id_to_hd_driveid(u16 *id)
	{
	#ifdef __BIG_ENDIAN

	..

	*(u64 *)&id[ATA_ID_LBA_CAPACITY_2] =
		ata_id_u64(id, ATA_ID_LBA_CAPACITY_2);

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

From: Christian Engelmayer <[email protected]>

This patch fixes a memory overrun in function ide_get_identity_ioctl() which chooses the size of a memory buffer depending on the ioctl command that led to the function call, however, passes that buffer to a function which needs the buffer size to be always chosen unconditionally.

Due to conditional compilation the memory overrun can only happen on big endian machines. The error can be triggered using ioctl HDIO_OBSOLETE_IDENTITY. Usage of ioctl HDIO_GET_IDENTITY is safe.

Signed-off-by Christian Engelmayer <[email protected]>

--
Proposed patch after comment by Robert Hancock who shares the view that buffer 'id' should be allocated unconditionally.

ide/ide-ioctls.c renamed from ide/ide-ioctls.c.orig +2 -1

@@ -64,7 +64,8 @@ static int ide_get_identity_ioctl(ide_dr
 		goto out;
 	}
 
-	id = kmalloc(size, GFP_KERNEL);
+	/* ata_id_to_hd_driveid() relies on 'id' to be fully allocated. */
+	id = kmalloc(ATA_ID_WORDS * 2, GFP_KERNEL);
 	if (id == NULL) {
 		rc = -ENOMEM;
 		goto out;

Quoted 7 lines

From: Christian Engelmayer <[email protected]>

This patch fixes a memory overrun in function ide_get_identity_ioctl() which chooses the size of a memory buffer depending on the ioctl command that led to the function call, however, passes that buffer to a function which needs the buffer size to be always chosen unconditionally.

Quoted 5 lines

Due to conditional compilation the memory overrun can only happen on big endian machines. The error can be triggered using ioctl HDIO_OBSOLETE_IDENTITY. Usage of ioctl HDIO_GET_IDENTITY is safe.

Signed-off-by Christian Engelmayer <[email protected]>

Acked-by Bartlomiej Zolnierkiewicz <[email protected]>