[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 2/2] vvfat: Fix array_remove_slice()
From: |
Eric Blake |
Subject: |
Re: [PATCH 2/2] vvfat: Fix array_remove_slice() |
Date: |
Tue, 23 Jun 2020 13:30:04 -0500 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.9.0 |
On 6/23/20 12:55 PM, Kevin Wolf wrote:
array_remove_slice() calls array_roll() with array->next - 1 as the
destination index. This is only correct for count == 1, otherwise we're
writing past the end of the array. array->next - count would be correct.
However, this is the only place ever calling array_roll(), so this
rather complicated operation isn't even necessary.
Fix the problem and simplify the code by replacing it with a single
memmove() call. array_roll() can now be removed.
Reported-by: Nathan Huckleberry <nhuck15@gmail.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
block/vvfat.c | 42 +++++-------------------------------------
1 file changed, 5 insertions(+), 37 deletions(-)
diff --git a/block/vvfat.c b/block/vvfat.c
index 2fab371258..d6e464c595 100644
--- a/block/vvfat.c
+++ b/block/vvfat.c
@@ -140,48 +140,16 @@ static inline void* array_insert(array_t* array,unsigned
int index,unsigned int
return array->pointer+index*array->item_size;
}
-/* this performs a "roll", so that the element which was at index_from becomes
- * index_to, but the order of all other elements is preserved. */
-static inline int array_roll(array_t* array,int index_to,int index_from,int
count)
If I understand the intent from just the comment, the old code would
take a directory listing of six files:
ABCDEF
and on the request to delete file C, would produce:
ABFDE
by moving just F, instead of all of DEF. That might be what legacy FAT
filesystems do natively, but I don't see it as a mandatory correctness
issue; ABDEF is still a directory with the same contents. And the bug
for reading beyond array bounds when deleting more than one file is
indeed nasty, so your simpler code is fine.
static inline int array_remove_slice(array_t* array,int index, int count)
{
assert(index >=0);
assert(count > 0);
assert(index + count <= array->next);
- if(array_roll(array,array->next-1,index,count))
- return -1;
+
+ memmove(array->pointer + index * array->item_size,
+ array->pointer + (index + count) * array->item_size,
+ (array->next - index - count) * array->item_size);
+
Reviewed-by: Eric Blake <eblake@redhat.com>
array->next -= count;
return 0;
}
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org