Re: [PATCH] Improve support for mac partition tables with logical sector size != 512

[Peter Jones]

On 01/03/2011 05:49 PM, Peter Jones wrote:
> On 12/17/2010 02:32 PM, Jim Meyering wrote:
>> Brian C. Lane wrote:
>>> From: Peter Jones <address@hidden>
>>>
>>> On mac partition tables which specify a sector size larger than the
>>> physical sector size, we need to reallocate the buffer after we
>>> determine the correct sector size.
>>>
>>> This is a normal condition when CD is used with Apple partitions (for
>>> example, the USB rescue stick for the MacBookAir3,1), and then an image
>>> (analogous to an .iso image) is created, so also don't raise an exception in
>>> _disk_analyse_block_size() when we find that.
>>>
>>> Also simplify the code in _disk_analyse_block_size() a bit since there's no
>>> reason to ever do the byteswapping more than once or convert everything to
>>> 512-byte multiples since we neither load nor store it that way.
>>> ---
>>>  libparted/labels/mac.c |   44 ++++++++++++++++++++++++++------------------
>>>  1 files changed, 26 insertions(+), 18 deletions(-)
>>>
>>> diff --git a/libparted/labels/mac.c b/libparted/labels/mac.c
>>
>> Hi Brian,
>>
>> Thanks for passing that along.
>> Thanks to Peter for writing the patch.
>>
>> Is there a bugzilla number for that, or a test case?
>> I presume the existing code is causing some sort of failure.
>> If so, can you give details, or even outline how to provoke failure?
>> I'd really like to get enough information that I can reproduce
>> the failure and write a test case.
> 
> The test case is:
> 
> 1) Get a new macbook air
> 2) find the tiny little USB stick in the box with the restore image on it
> 3) plug the stick in (it'll be seen as a CD device, so probably sr0 or sr1)
> 4) dd if=/dev/sr1 of=mac.img
> 5) losetup /dev/loop0 mac.img
> 6) parted /dev/loop0 p
> 
> at which point it segfaults.  What seems to happen is that ptt_read_sector()
> allocates a buffer with the default size of 512.  Then we call 
> _disk_analyse_block_size(), which correctly sets the (logical) sector size to
> 2048, which is what's set in the label's data.  After that, everything expects
> the buffer to be 2048 bytes, because that's what the sector size is set at,
> but the buffer is still the original 512 byte allocation.

One thing I should have said here and didn't - the bug specifically happens
because the image was originally created with a sector size of 2048 - the CD
device. So if you just run parted on a CD formatted this way, it should
work. But once we've got it on a loopback device, the hardware sector size is
512, which is where ptt_disk_read()'s initial allocation size is coming from.

-- 
        Peter