linux-kernel - Re: [PATCH 06/22] ide-tape: struct idetape_tape

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <48D7BEDE.5040408@ru.mvista.com>
Date:	Mon, 22 Sep 2008 19:50:54 +0400
From:	Sergei Shtylyov <sshtylyov@...mvista.com>
To:	petkovbb@...il.com
Cc:	bzolnier@...il.com, linux-kernel@...r.kernel.org,
	linux-ide@...r.kernel.org
Subject: Re: [PATCH 06/22] ide-tape: struct idetape_tape_t: remove unused
 members

Boris Petkov wrote:

>>>@@ -3438,9 +3419,9 @@ static int idetape_identify_device (ide_drive_t
>>>*drive)
>>>  static void idetape_get_inquiry_results(ide_drive_t *drive)
>>> {
>>>-       char *r;
>>>       idetape_tape_t *tape = drive->driver_data;
>>>       idetape_pc_t pc;
>>>+       char fw_rev[6], vendor_id[10], product_id[18];
>>>         idetape_create_inquiry_cmd(&pc);
>>>       if (idetape_queue_pc_tail(drive, &pc)) {
>>>@@ -3448,20 +3429,16 @@ static void
>>>idetape_get_inquiry_results(ide_drive_t *drive)
>>>                               tape->name);
>>>               return;
>>>       }
>>>-       memcpy(tape->vendor_id, &pc.buffer[8], 8);
>>>-       memcpy(tape->product_id, &pc.buffer[16], 16);
>>>-       memcpy(tape->firmware_revision, &pc.buffer[32], 4);
>>>-
>>>-       ide_fixstring(tape->vendor_id, 10, 0);
>>>-       ide_fixstring(tape->product_id, 18, 0);
>>>-       ide_fixstring(tape->firmware_revision, 6, 0);
>>>-       r = tape->firmware_revision;
>>>-       if (*(r + 1) == '.')
>>>-               tape->firmware_revision_num = (*r - '0') * 100 +
>>>-                       (*(r + 2) - '0') * 10 + *(r + 3) - '0';
>>>+       memcpy(vendor_id, &pc.buffer[8], 8);
>>>+       memcpy(product_id, &pc.buffer[16], 16);
>>>+       memcpy(fw_rev, &pc.buffer[32], 4);
>>>+
>>>+       ide_fixstring(vendor_id, 10, 0);
>>>+       ide_fixstring(product_id, 18, 0);
>>>+       ide_fixstring(fw_rev, 6, 0);

>>  It was wrong to call ide_fixstring() on unterminated strings and expecting
>>them to become terminated strings after that; plus it was useless to add 2
>>characters padding at the end. When these variables were the fields of
>>'struct ide_tape_obj', those bytes were 0 because of the variable of this
>>type being a static array. When they became local variables, they got
>>garbage bytes at the end which ide_fixdriveid() either honestly copied when
>>compressing spaces or just left where they were...

> LOL, i just sent out a similar fix :).

>>>+
>>>       printk(KERN_INFO "ide-tape: %s <-> %s: %s %s rev %s\n",

>>  Should've rather changed the string format to print only N characters
>>max...

    The fix that I have suggested isn't at all similar.  We don't need to 
waste memory on extra bytes, and even less on them being of the 'static' 
memory class...

MBR, Sergei
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/