Messages in this thread | | | From | Jeff Moyer <> | Date | Mon, 13 Feb 2006 18:09:55 -0500 | Subject | Re: [patch] fix BUG: in fw_realloc_buffer |
| |
==> Regarding Re: [patch] fix BUG: in fw_realloc_buffer; Jeff Moyer <jmoyer@redhat.com> adds:
==> Regarding Re: [patch] fix BUG: in fw_realloc_buffer; Andrew Morton <akpm@osdl.org> adds: akpm> Jeff Moyer <jmoyer@redhat.com> wrote: >>> >>> Hi, >>> >>> The fw_realloc_buffer routine does not handle an increase in buffer size of >>> more than 4k. It's not clear to me why it expects that it will only get an >>> extra 4k of data. The attached patch modifies fw_realloc_buffer to vmalloc >>> as much memory as is requested, instead of what we previously had + 4k. >>> >>> I've tested this on my laptop, which would crash occaisionally on boot >>> without the patch. With the patch, it hasn't crashed, but I can't be >>> certain that this code path is exercised. >>> >>> Comments are very welcome. >>> jmoyer> [snip]
akpm> A little bit neater this way, I think?
>> --- devel/drivers/base/firmware_class.c~firmware-fix-bug-in-fw_realloc_buffer 2006-02-13 14:45:52.000000000 -0800 >> +++ devel-akpm/drivers/base/firmware_class.c 2006-02-13 14:52:05.000000000 -0800 >> @@ -211,18 +211,20 @@ static int >> fw_realloc_buffer(struct firmware_priv *fw_priv, int min_size) >> { >> u8 *new_data; >> + int new_size = fw_priv->alloc_size;
>> if (min_size <= fw_priv->alloc_size) >> return 0;
>> - new_data = vmalloc(fw_priv->alloc_size + PAGE_SIZE); >> + new_size = ALIGN(min_size, PAGE_SIZE); >> + new_data = vmalloc(new_size); >> if (!new_data) { >> printk(KERN_ERR "%s: unable to alloc buffer\n", __FUNCTION__); >> /* Make sure that we don't keep incomplete data */ >> fw_load_abort(fw_priv); >> return -ENOMEM; >> } >> - fw_priv->alloc_size += PAGE_SIZE; >> + fw_priv->alloc_size = new_size; >> if (fw_priv->fw->data) { >> memcpy(new_data, fw_priv->fw->data, fw_priv->fw->size); >> vfree(fw_priv->fw->data); >> _
jmoyer> Well, I wasn't sure that you would only need to increase by a PAGE. If you jmoyer> only need to account for page_size + alignment, then yes, this is better. jmoyer> It simply wasn't clear to me that this is how we are called. If I'm not jmoyer> mistaken, this is the write routine for a file in sysfs. If that is the jmoyer> case, why should we assume that writes are broken up into PAGE_SIZE chunks?
Doh, I wasn't looking close enough at this. Yes, your fix is what was intended. Thanks.
-Jeff - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |