Consider increasing the assertion that HDD serial numbers are limited to 20 bytes. #547
Comments
|
Have you still got that drive available? If so any chance you can download the master again, once I've patched it this afternoon and try again. While the increase to 30 might fix it, the ata specs state 20 character string for serial number. There is a strcpy in the code which I've just replaced, maybe that was overflowing. Can you post the nwipe log and the serial number of the drive. Seems real odd to have a load of white space in the middle. They are supposed to left justify the serial number and pad with spaces to 20 characters if they following the ata standard. |
|
Here is a log directly from nwipe and a stack trace. Here are a few serial numbers. Like I said, I bought them from a liquidator, so I can only assume that either they themselves, or whatever vendor originally produced the setup they liquidated are at fault for this. I've bought plenty of other drives from them that had normal serial numbers, but a particular series of drives of the same model all seem to have this odd pattern. And I can't ask them as they are now defunct. |
The strings such as model and serial number stored in a drive are not generally null terminated in the drive. So for instance the 20 characters allocated in the drive for serial number could be immediately followed by some other string, with no separator so by expanding the storage in nwipe to 30 characters you are just reading some other field. What I need to determine is why it would buffer overflow with the serial number set to 20+1 (the +1 is for the terminator.) That strace was very useful, thanks. I've changed a couple of strcpys to strncpys that might have caused a buffer overflow, however, the nwipe log showed that you were running 0.35 and a commit #527 (0.35.1+) would have fixed a number of potential buffer overflow issues in exactly the area of code that the strace indicated. Can you try downloading the latest master version 0.35.9 and see if it still segfaults. I'm expecting it to work fine, but we'll see. Thanks. |
|
Indeed, the current master does not overflow. However, the view of the serial number field is truncated compared to the modification I made. Certainly a much less severe issue!
|
|
Is there any evidence to suggest that 2EGKYRHV is part of the serial number? The vendor has padded the first number with spaces to what looks like 20 characters, which would be consistent with how the serial is supposed to be written according to the SCSI command reference. If it doesn't occupy all 20 characters it should be right padded with spaces. If no serial number all 20 characters should be spaces. What does smartctl -a /dev/sde recognise the serial number as? |
|
That 2EGKYRHV looks like it could be the last 8 digits of another drive you were wiping at the same time? Strange that the last 5 digits of both numbers are the same. |
|
Looks like I accidentally copied the same string twice. The first part is the same too! Smartctl lists the entire given string in the serial number field. The people I bought them from also listed the extranous characters as the drive's serial numbers on the packaging that they were shipped in.
|
|
Can you also post the output of Thanks. |
|
Sorry, correction to my previous message, can you run this command |
|
Well, with hdparm I get nothing useful. It seems hdparm doesn't work with SAS drives. However, sgdisk does return the same information as smartctl. |
|
Interesting, thanks, unfortunately re sginfo -A, although it does contain hex data, it doesn't provide the level of detail hdparm --verbose does, unless sginfo has a similar option that outputs the real low level command and return data. It is interesting that it also provides the same serial number result which seems to indicate that the field length is defined as larger than 20 characters. I was looking for an example of a H7280A520SUN8.0T online to see what the typical serial number looks like and found one below, has a 18 character serial number. HUH model but half way down states it's a H720. If there are other drives that exceed 20 character serial numbers I may have to consider a variable length. As these drives you have are not necessarily a typical H7280.. I'm not too concerned. However changing the serial number length to variable may happen. I need to look at the ata command specs. I've got a feeling it does provide a length field for serial number but need to double check that.
|
As described in this line, HDD serial numbers may be no more than 20 bytes. If this constraint is violated, nwipe will exit with a buffer overflow.
nwipe/src/context.h
Line 83 in 2fae6ea
I have some disks from a dodgy liquidation store that seems to have attempted to flash their own serial numbers onto the drives, resulting in a serial number appended to another serial number with a rather large whitespace gap between them, that doesn't get trimmed by the current code, that is 30 bytes long in total. Increasing this one line from 20 to 30 and recompiling fixed the issue on my machine. Probably a pretty rare problem to have, and idk if there's any performance/other reason to keep it set at 20, but this is a thing that just happened to me.
The text was updated successfully, but these errors were encountered: