Re: port-evbarm/54554

[Simon South <simon%simonsouth.net@localhost>]

The following reply was made to PR port-evbarm/54554; it has been noted by GNATS.

From: Simon South <simon%simonsouth.net@localhost>
To: gnats-bugs%NetBSD.org@localhost
Cc: 
Subject: Re: port-evbarm/54554
Date: Tue, 17 Sep 2019 11:27:50 -0400

 This is a multi-part message in MIME format.
 --------------42AFB12FB7BE06A1DACFB984
 Content-Type: text/plain; charset=utf-8; format=flowed
 Content-Transfer-Encoding: 7bit
 
 Here's a patch that fixes this problem by ensuring calls to ReadBlocks 
 in efiblock.c always pass a buffer aligned as requested by the device.
 
 In the case of efi_block_strategy(), this is done by checking the 
 alignment of the buffer passed by the caller and, if necessary, 
 allocating a second buffer, writing into it at the correct alignment and 
 copying the result back into the caller's buffer.
 
 Note I've taken the liberty of wrapping at 80 columns the lines I modified.
 
 I've tested this successfully on a PINE64 ROCK64 (only).
 
 -- 
 Simon South
 simon%simonsouth.net@localhost
 
 --------------42AFB12FB7BE06A1DACFB984
 Content-Type: text/x-patch; charset=UTF-8;
  name="54554-efiboot-align-buffers.patch"
 Content-Transfer-Encoding: 7bit
 Content-Disposition: attachment;
  filename="54554-efiboot-align-buffers.patch"
 
 Index: sys/stand/efiboot/efiblock.c
 ===================================================================
 RCS file: /cvsroot/src/sys/stand/efiboot/efiblock.c,v
 retrieving revision 1.5
 diff -u -r1.5 efiblock.c
 --- sys/stand/efiboot/efiblock.c	9 Mar 2019 13:16:42 -0000	1.5
 +++ sys/stand/efiboot/efiblock.c	17 Sep 2019 15:23:59 -0000
 @@ -106,19 +106,22 @@
  	struct partition *p;
  	EFI_STATUS status;
  	EFI_LBA lba;
 -	uint8_t *buf;
 +	uint8_t *buf, *aligned_buf;
  	UINT32 sz;
  	int n;
  
  	sz = __MAX(sizeof(d), bdev->bio->Media->BlockSize);
  	sz = roundup(sz, bdev->bio->Media->BlockSize);
 -	buf = AllocatePool(sz);
 +	buf = AllocatePool(sz + bdev->bio->Media->IoAlign - 1);
  	if (!buf)
  		return ENOMEM;
 +	aligned_buf = (uint8_t *)(((intptr_t)buf + bdev->bio->Media->IoAlign - 1) &
 +		~(bdev->bio->Media->IoAlign - 1));
  
  	lba = (((EFI_LBA)start + LABELSECTOR) * DEV_BSIZE) / bdev->bio->Media->BlockSize;
 -	status = uefi_call_wrapper(bdev->bio->ReadBlocks, 5, bdev->bio, bdev->media_id, lba, sz, buf);
 -	if (EFI_ERROR(status) || getdisklabel(buf, &d) != NULL) {
 +	status = uefi_call_wrapper(bdev->bio->ReadBlocks, 5, bdev->bio, bdev->media_id,
 +		lba, sz, aligned_buf);
 +	if (EFI_ERROR(status) || getdisklabel(aligned_buf, &d) != NULL) {
  		FreePool(buf);
  		return EIO;
  	}
 @@ -159,22 +162,25 @@
  	struct mbr_sector mbr;
  	struct mbr_partition *mbr_part;
  	EFI_STATUS status;
 -	uint8_t *buf;
 +	uint8_t *buf, *aligned_buf;
  	UINT32 sz;
  	int n;
  
  	sz = __MAX(sizeof(mbr), bdev->bio->Media->BlockSize);
  	sz = roundup(sz, bdev->bio->Media->BlockSize);
 -	buf = AllocatePool(sz);
 +	buf = AllocatePool(sz + bdev->bio->Media->IoAlign - 1);
  	if (!buf)
  		return ENOMEM;
 +	aligned_buf = (uint8_t *)(((intptr_t)buf + bdev->bio->Media->IoAlign - 1) &
 +		~(bdev->bio->Media->IoAlign - 1));
  
 -	status = uefi_call_wrapper(bdev->bio->ReadBlocks, 5, bdev->bio, bdev->media_id, 0, sz, buf);
 +	status = uefi_call_wrapper(bdev->bio->ReadBlocks, 5, bdev->bio, bdev->media_id,
 +		0, sz, aligned_buf);
  	if (EFI_ERROR(status)) {
  		FreePool(buf);
  		return EIO;
  	}
 -	memcpy(&mbr, buf, sizeof(mbr));
 +	memcpy(&mbr, aligned_buf, sizeof(mbr));
  	FreePool(buf);
  
  	if (le32toh(mbr.mbr_magic) != MBR_MAGIC)
 @@ -241,20 +247,23 @@
  	struct gpt_ent ent;
  	EFI_STATUS status;
  	UINT32 sz, entry;
 -	uint8_t *buf;
 +	uint8_t *buf, *aligned_buf;
  
  	sz = __MAX(sizeof(hdr), bdev->bio->Media->BlockSize);
  	sz = roundup(sz, bdev->bio->Media->BlockSize);
 -	buf = AllocatePool(sz);
 +	buf = AllocatePool(sz + bdev->bio->Media->IoAlign - 1);
  	if (!buf)
  		return ENOMEM;
 +	aligned_buf = (uint8_t *)(((intptr_t)buf + bdev->bio->Media->IoAlign - 1) &
 +		~(bdev->bio->Media->IoAlign - 1));
  
 -	status = uefi_call_wrapper(bdev->bio->ReadBlocks, 5, bdev->bio, bdev->media_id, GPT_HDR_BLKNO, sz, buf);
 +	status = uefi_call_wrapper(bdev->bio->ReadBlocks, 5, bdev->bio, bdev->media_id,
 +		GPT_HDR_BLKNO, sz, aligned_buf);
  	if (EFI_ERROR(status)) {
  		FreePool(buf);
  		return EIO;
  	}
 -	memcpy(&hdr, buf, sizeof(hdr));
 +	memcpy(&hdr, aligned_buf, sizeof(hdr));
  	FreePool(buf);
  
  	if (memcmp(hdr.hdr_sig, GPT_HDR_SIG, sizeof(hdr.hdr_sig)) != 0)
 @@ -264,18 +273,22 @@
  
  	sz = __MAX(le32toh(hdr.hdr_entsz) * le32toh(hdr.hdr_entries), bdev->bio->Media->BlockSize);
  	sz = roundup(sz, bdev->bio->Media->BlockSize);
 -	buf = AllocatePool(sz);
 +	buf = AllocatePool(sz + bdev->bio->Media->IoAlign - 1);
  	if (!buf)
  		return ENOMEM;
 +	aligned_buf = (uint8_t *)(((intptr_t)buf + bdev->bio->Media->IoAlign - 1) &
 +		~(bdev->bio->Media->IoAlign - 1));
  
 -	status = uefi_call_wrapper(bdev->bio->ReadBlocks, 5, bdev->bio, bdev->media_id, le64toh(hdr.hdr_lba_table), sz, buf);
 +	status = uefi_call_wrapper(bdev->bio->ReadBlocks, 5, bdev->bio, bdev->media_id,
 +		le64toh(hdr.hdr_lba_table), sz, aligned_buf);
  	if (EFI_ERROR(status)) {
  		FreePool(buf);
  		return EIO;
  	}
  
  	for (entry = 0; entry < le32toh(hdr.hdr_entries); entry++) {
 -		memcpy(&ent, buf + (entry * le32toh(hdr.hdr_entsz)), sizeof(ent));
 +		memcpy(&ent, aligned_buf + (entry * le32toh(hdr.hdr_entsz)),
 +			sizeof(ent));
  		efi_block_find_partitions_gpt_entry(bdev, &hdr, &ent, entry);
  	}
  
 @@ -493,6 +506,7 @@
  {
  	struct efi_block_part *bpart = devdata;
  	EFI_STATUS status;
 +	void *allocated_buf, *aligned_buf;
  
  	if (rw != F_READ)
  		return EROFS;
 @@ -518,9 +532,30 @@
  		return EINVAL;
  	}
  
 -	status = uefi_call_wrapper(bpart->bdev->bio->ReadBlocks, 5, bpart->bdev->bio, bpart->bdev->media_id, dblk, size, buf);
 -	if (EFI_ERROR(status))
 +	if (((intptr_t)buf & ~(bpart->bdev->bio->Media->IoAlign - 1)) == 0) {
 +		allocated_buf = NULL;
 +		aligned_buf = buf;
 +	} else {
 +		allocated_buf = AllocatePool(size +
 +			bpart->bdev->bio->Media->IoAlign - 1);
 +		if (!allocated_buf)
 +			return ENOMEM;
 +		aligned_buf = (void *)(((intptr_t)allocated_buf +
 +			bpart->bdev->bio->Media->IoAlign - 1) &
 +			~(bpart->bdev->bio->Media->IoAlign - 1));
 +	}
 +
 +	status = uefi_call_wrapper(bpart->bdev->bio->ReadBlocks, 5,
 +		bpart->bdev->bio, bpart->bdev->media_id, dblk, size, aligned_buf);
 +	if (EFI_ERROR(status)) {
 +		if (allocated_buf)
 +			FreePool(allocated_buf);
  		return EIO;
 +	}
 +	if (allocated_buf) {
 +		memcpy(buf, aligned_buf, size);
 +		FreePool(allocated_buf);
 +	}
  
  	*rsize = size;
  
 
 --------------42AFB12FB7BE06A1DACFB984--