[U-Boot] [PATCH 01/10] usb: gadget: don't leak configs when unbinding
From: Stephen Warren swarren@nvidia.com
By the time g_dnl_unbind() is run, cdev->config has been set to NULL, so the free() there does nothing, and the config struct is leaked. Equally, struct usb_gadget contains a linked list of config structs, so the code should iterate over them all and free each one, rather than freeing one particular config struct.
composite_unbind() already iterates over the list of config structs, and unlinks each from the linked list. Fix this loop to free() each struct as it's unlinked and otherwise forgotten.
Signed-off-by: Stephen Warren swarren@nvidia.com --- drivers/usb/gadget/composite.c | 1 + drivers/usb/gadget/g_dnl.c | 2 -- 2 files changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c index d96296cd73b1..a13b21d0a0f2 100644 --- a/drivers/usb/gadget/composite.c +++ b/drivers/usb/gadget/composite.c @@ -948,6 +948,7 @@ static void composite_unbind(struct usb_gadget *gadget) debug("unbind config '%s'/%p\n", c->label, c); c->unbind(c); } + free(c); } if (composite->unbind) composite->unbind(cdev); diff --git a/drivers/usb/gadget/g_dnl.c b/drivers/usb/gadget/g_dnl.c index ad89a0d2e670..2fa6da4091e1 100644 --- a/drivers/usb/gadget/g_dnl.c +++ b/drivers/usb/gadget/g_dnl.c @@ -93,8 +93,6 @@ static int g_dnl_unbind(struct usb_composite_dev *cdev) { struct usb_gadget *gadget = cdev->gadget;
- free(cdev->config); - cdev->config = NULL; debug("%s: calling usb_gadget_disconnect for " "controller '%s'\n", __func__, gadget->name); usb_gadget_disconnect(gadget);
From: Stephen Warren swarren@nvidia.com
ext4_write_file() is only called from the "fs" layer, which calls both ext4fs_mount() and ext4fs_close() before/after calling ext4_write_file(). Fix ext4_write_file() not to call ext4fs_mount() again, since the mount operation malloc()s some RAM which is leaked when a second mount call over-writes the pointer to that data, if no intervening close call is made.
Signed-off-by: Stephen Warren swarren@nvidia.com --- fs/ext4/ext4_write.c | 9 --------- 1 file changed, 9 deletions(-)
diff --git a/fs/ext4/ext4_write.c b/fs/ext4/ext4_write.c index fbc4c4b1cc1a..fa67eb6a56b4 100644 --- a/fs/ext4/ext4_write.c +++ b/fs/ext4/ext4_write.c @@ -986,26 +986,17 @@ int ext4_write_file(const char *filename, void *buf, loff_t offset, return -1; }
- /* mount the filesystem */ - if (!ext4fs_mount(0)) { - printf("** Error Bad ext4 partition **\n"); - goto fail; - } - ret = ext4fs_write(filename, buf, len); - if (ret) { printf("** Error ext4fs_write() **\n"); goto fail; } - ext4fs_close();
*actwrite = len;
return 0;
fail: - ext4fs_close(); *actwrite = 0;
return -1;
Hi Stephen,
From: Stephen Warren swarren@nvidia.com
ext4_write_file() is only called from the "fs" layer, which calls both ext4fs_mount() and ext4fs_close() before/after calling ext4_write_file(). Fix ext4_write_file() not to call ext4fs_mount() again, since the mount operation malloc()s some RAM which is leaked when a second mount call over-writes the pointer to that data, if no intervening close call is made.
Signed-off-by: Stephen Warren swarren@nvidia.com
fs/ext4/ext4_write.c | 9 --------- 1 file changed, 9 deletions(-)
diff --git a/fs/ext4/ext4_write.c b/fs/ext4/ext4_write.c index fbc4c4b1cc1a..fa67eb6a56b4 100644 --- a/fs/ext4/ext4_write.c +++ b/fs/ext4/ext4_write.c @@ -986,26 +986,17 @@ int ext4_write_file(const char *filename, void *buf, loff_t offset, return -1; }
/* mount the filesystem */
if (!ext4fs_mount(0)) {
printf("** Error Bad ext4 partition **\n");goto fail;}
ret = ext4fs_write(filename, buf, len);
if (ret) { printf("** Error ext4fs_write() **\n"); goto fail; }
ext4fs_close();
*actwrite = len;
return 0;
fail:
ext4fs_close(); *actwrite = 0;
return -1;
Acked-by: Lukasz Majewski l.majewski@samsung.com Tested-by: Lukasz Majewski l.majewski@samsung.com
Test HW: Odroid XU3 - Exynos5433 [DFU tests]
On Fri, Sep 04, 2015 at 10:03:43PM -0600, Stephen Warren wrote:
From: Stephen Warren swarren@nvidia.com
ext4_write_file() is only called from the "fs" layer, which calls both ext4fs_mount() and ext4fs_close() before/after calling ext4_write_file(). Fix ext4_write_file() not to call ext4fs_mount() again, since the mount operation malloc()s some RAM which is leaked when a second mount call over-writes the pointer to that data, if no intervening close call is made.
Signed-off-by: Stephen Warren swarren@nvidia.com Acked-by: Lukasz Majewski l.majewski@samsung.com Tested-by: Lukasz Majewski l.majewski@samsung.com
Applied to u-boot/master, thanks!
From: Stephen Warren swarren@nvidia.com
parse_path() malloc()s the entries in the array it's passed. Those allocations must be free()d by the caller, ext4fs_get_parent_inode_num(). Add code to do this.
For this to work, all the array entries must be dynamically allocated, rather than a mix of dynamic and static allocations. Fix parse_path() not to over-write arr[0] with a pointer to statically allocated data.
Signed-off-by: Stephen Warren swarren@nvidia.com --- fs/ext4/ext4_common.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/fs/ext4/ext4_common.c b/fs/ext4/ext4_common.c index cab5465b9d4f..b09f23aa5b83 100644 --- a/fs/ext4/ext4_common.c +++ b/fs/ext4/ext4_common.c @@ -614,8 +614,7 @@ static int parse_path(char **arr, char *dirname) arr[i] = zalloc(strlen("/") + 1); if (!arr[i]) return -ENOMEM; - - arr[i++] = "/"; + memcpy(arr[i++], "/", strlen("/"));
/* add each path entry after root */ while (token != NULL) { @@ -745,6 +744,11 @@ end: fail: free(depth_dirname); free(parse_dirname); + for (i = 0; i < depth; i++) { + if (!ptr[i]) + break; + free(ptr[i]); + } free(ptr); free(parent_inode); free(first_inode);
Hi Stephen,
From: Stephen Warren swarren@nvidia.com
parse_path() malloc()s the entries in the array it's passed. Those allocations must be free()d by the caller, ext4fs_get_parent_inode_num(). Add code to do this.
For this to work, all the array entries must be dynamically allocated, rather than a mix of dynamic and static allocations. Fix parse_path() not to over-write arr[0] with a pointer to statically allocated data.
Signed-off-by: Stephen Warren swarren@nvidia.com
fs/ext4/ext4_common.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/fs/ext4/ext4_common.c b/fs/ext4/ext4_common.c index cab5465b9d4f..b09f23aa5b83 100644 --- a/fs/ext4/ext4_common.c +++ b/fs/ext4/ext4_common.c @@ -614,8 +614,7 @@ static int parse_path(char **arr, char *dirname) arr[i] = zalloc(strlen("/") + 1); if (!arr[i]) return -ENOMEM;
- arr[i++] = "/";
memcpy(arr[i++], "/", strlen("/"));
/* add each path entry after root */ while (token != NULL) {
@@ -745,6 +744,11 @@ end: fail: free(depth_dirname); free(parse_dirname);
- for (i = 0; i < depth; i++) {
if (!ptr[i])break;free(ptr[i]);- } free(ptr); free(parent_inode); free(first_inode);
Acked-by: Lukasz Majewski l.majewski@samsung.com Tested-by: Lukasz Majewski l.majewski@samsung.com
Test HW: Odroid XU3 - Exynos5433 [DFU tests]
On Fri, Sep 04, 2015 at 10:03:44PM -0600, Stephen Warren wrote:
From: Stephen Warren swarren@nvidia.com
parse_path() malloc()s the entries in the array it's passed. Those allocations must be free()d by the caller, ext4fs_get_parent_inode_num(). Add code to do this.
For this to work, all the array entries must be dynamically allocated, rather than a mix of dynamic and static allocations. Fix parse_path() not to over-write arr[0] with a pointer to statically allocated data.
Signed-off-by: Stephen Warren swarren@nvidia.com Acked-by: Lukasz Majewski l.majewski@samsung.com Tested-by: Lukasz Majewski l.majewski@samsung.com
Applied to u-boot/master, thanks!
From: Stephen Warren swarren@nvidia.com
root_first_block_buffer should be free()d in all cases, not just when an error occurs. Fix the success exit path of the function to do this.
Signed-off-by: Stephen Warren swarren@nvidia.com --- fs/ext4/ext4_common.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/fs/ext4/ext4_common.c b/fs/ext4/ext4_common.c index b09f23aa5b83..7d7609a91fa8 100644 --- a/fs/ext4/ext4_common.c +++ b/fs/ext4/ext4_common.c @@ -769,6 +769,7 @@ static int check_filename(char *filename, unsigned int blknr) struct ext2_dirent *previous_dir = NULL; char *ptr = NULL; struct ext_filesystem *fs = get_fs(); + int ret = -1;
/* get the first block of root */ first_block_no_of_root = blknr; @@ -822,12 +823,12 @@ static int check_filename(char *filename, unsigned int blknr) if (ext4fs_put_metadata(root_first_block_addr, first_block_no_of_root)) goto fail; - return inodeno; + ret = inodeno; } fail: free(root_first_block_buffer);
- return -1; + return ret; }
int ext4fs_filename_check(char *filename)
Hi Stephen,
From: Stephen Warren swarren@nvidia.com
root_first_block_buffer should be free()d in all cases, not just when an error occurs. Fix the success exit path of the function to do this.
Signed-off-by: Stephen Warren swarren@nvidia.com
fs/ext4/ext4_common.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/fs/ext4/ext4_common.c b/fs/ext4/ext4_common.c index b09f23aa5b83..7d7609a91fa8 100644 --- a/fs/ext4/ext4_common.c +++ b/fs/ext4/ext4_common.c @@ -769,6 +769,7 @@ static int check_filename(char *filename, unsigned int blknr) struct ext2_dirent *previous_dir = NULL; char *ptr = NULL; struct ext_filesystem *fs = get_fs();
int ret = -1;
/* get the first block of root */ first_block_no_of_root = blknr;
@@ -822,12 +823,12 @@ static int check_filename(char *filename, unsigned int blknr) if (ext4fs_put_metadata(root_first_block_addr, first_block_no_of_root)) goto fail;
return inodeno;
ret = inodeno;fail: free(root_first_block_buffer);
- return -1;
- return ret;
}
int ext4fs_filename_check(char *filename)
Acked-by: Lukasz Majewski l.majewski@samsung.com Tested-by: Lukasz Majewski l.majewski@samsung.com
Test HW: Odroid XU3 - Exynos5433 [DFU tests]
On Fri, Sep 04, 2015 at 10:03:45PM -0600, Stephen Warren wrote:
From: Stephen Warren swarren@nvidia.com
root_first_block_buffer should be free()d in all cases, not just when an error occurs. Fix the success exit path of the function to do this.
Signed-off-by: Stephen Warren swarren@nvidia.com Acked-by: Lukasz Majewski l.majewski@samsung.com Tested-by: Lukasz Majewski l.majewski@samsung.com
Applied to u-boot/master, thanks!
From: Stephen Warren swarren@nvidia.com
DFU currently allocates buffer memory at the start of each data transfer operation and frees it at the end. Especially since memalign() is used to allocate the buffer, and various other allocations happen during the transfer, this can expose the code to heap fragmentation, which prevents the allocation from succeeding on subsequent transfers.
Fix the code to allocate the buffer once when DFU mode is initialized, and free the buffer once when DFU mode is exited, to reduce the exposure to heap fragmentation.
The failure mode is:
// Internally to memalign(), this allocates a lot more than s to guarantee // that alignment can occur, then returns chunks of memory at the start/ // end of the allocated buffer to the heap. p = memalign(a, s); // Various other malloc()s occur here, some of which allocate the RAM // immediately before/after "p". // // DFU transfer is complete, so buffer is released. free(p); // By chance, no other malloc()/free() here, in DFU at least. // // A new DFU transfer starts, so the buffer is allocated again. // In theory this should succeed since we just free()d a buffer of the // same size. However, this fails because memalign() internally attempts // to allocate much more than "s", yet free(p) above only free()d a // little more than "s". p = memalign(a, s);
Signed-off-by: Stephen Warren swarren@nvidia.com --- drivers/dfu/dfu.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/dfu/dfu.c b/drivers/dfu/dfu.c index 675162d927d8..d85d3f507a7b 100644 --- a/drivers/dfu/dfu.c +++ b/drivers/dfu/dfu.c @@ -164,7 +164,6 @@ static int dfu_write_buffer_drain(struct dfu_entity *dfu) void dfu_write_transaction_cleanup(struct dfu_entity *dfu) { /* clear everything */ - dfu_free_buf(); dfu->crc = 0; dfu->offset = 0; dfu->i_blk_seq_num = 0; @@ -385,7 +384,6 @@ int dfu_read(struct dfu_entity *dfu, void *buf, int size, int blk_seq_num) dfu_hash_algo->name, dfu->crc); puts("\nUPLOAD ... done\nCtrl+C to exit ...\n");
- dfu_free_buf(); dfu->i_blk_seq_num = 0; dfu->crc = 0; dfu->offset = 0; @@ -433,6 +431,7 @@ static int dfu_fill_entity(struct dfu_entity *dfu, char *s, int alt, __func__, interface); return -1; } + dfu_get_buf(dfu);
return 0; } @@ -441,6 +440,7 @@ void dfu_free_entities(void) { struct dfu_entity *dfu, *p, *t = NULL;
+ dfu_free_buf(); list_for_each_entry_safe_reverse(dfu, p, &dfu_list, list) { list_del(&dfu->list); if (dfu->free_entity)
Hi Stephen,
From: Stephen Warren swarren@nvidia.com
DFU currently allocates buffer memory at the start of each data transfer operation and frees it at the end. Especially since memalign() is used to allocate the buffer, and various other allocations happen during the transfer, this can expose the code to heap fragmentation, which prevents the allocation from succeeding on subsequent transfers.
Fix the code to allocate the buffer once when DFU mode is initialized, and free the buffer once when DFU mode is exited, to reduce the exposure to heap fragmentation.
The failure mode is:
// Internally to memalign(), this allocates a lot more than s to guarantee // that alignment can occur, then returns chunks of memory at the start/ // end of the allocated buffer to the heap. p = memalign(a, s); // Various other malloc()s occur here, some of which allocate the RAM // immediately before/after "p". // // DFU transfer is complete, so buffer is released. free(p); // By chance, no other malloc()/free() here, in DFU at least. // // A new DFU transfer starts, so the buffer is allocated again. // In theory this should succeed since we just free()d a buffer of the // same size. However, this fails because memalign() internally attempts // to allocate much more than "s", yet free(p) above only free()d a // little more than "s". p = memalign(a, s);
Signed-off-by: Stephen Warren swarren@nvidia.com
drivers/dfu/dfu.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/dfu/dfu.c b/drivers/dfu/dfu.c index 675162d927d8..d85d3f507a7b 100644 --- a/drivers/dfu/dfu.c +++ b/drivers/dfu/dfu.c @@ -164,7 +164,6 @@ static int dfu_write_buffer_drain(struct dfu_entity *dfu) void dfu_write_transaction_cleanup(struct dfu_entity *dfu) { /* clear everything */
- dfu_free_buf(); dfu->crc = 0; dfu->offset = 0; dfu->i_blk_seq_num = 0;
@@ -385,7 +384,6 @@ int dfu_read(struct dfu_entity *dfu, void *buf, int size, int blk_seq_num) dfu_hash_algo->name, dfu->crc); puts("\nUPLOAD ... done\nCtrl+C to exit ...\n");
dfu_free_buf();@@ -433,6 +431,7 @@ static int dfu_fill_entity(struct dfu_entity *dfu, char *s, int alt, __func__, interface); return -1; }
dfu_get_buf(dfu);
return 0;
} @@ -441,6 +440,7 @@ void dfu_free_entities(void) { struct dfu_entity *dfu, *p, *t = NULL;
- dfu_free_buf(); list_for_each_entry_safe_reverse(dfu, p, &dfu_list, list) { list_del(&dfu->list); if (dfu->free_entity)
Acked-by: Lukasz Majewski l.majewski@samsung.com Tested-by: Lukasz Majewski l.majewski@samsung.com
Test HW: Odroid XU3 - Exynos5433 [DFU tests]
On Fri, Sep 04, 2015 at 10:03:46PM -0600, Stephen Warren wrote:
From: Stephen Warren swarren@nvidia.com
DFU currently allocates buffer memory at the start of each data transfer operation and frees it at the end. Especially since memalign() is used to allocate the buffer, and various other allocations happen during the transfer, this can expose the code to heap fragmentation, which prevents the allocation from succeeding on subsequent transfers.
Fix the code to allocate the buffer once when DFU mode is initialized, and free the buffer once when DFU mode is exited, to reduce the exposure to heap fragmentation.
The failure mode is:
// Internally to memalign(), this allocates a lot more than s to guarantee // that alignment can occur, then returns chunks of memory at the start/ // end of the allocated buffer to the heap. p = memalign(a, s); // Various other malloc()s occur here, some of which allocate the RAM // immediately before/after "p". // // DFU transfer is complete, so buffer is released. free(p); // By chance, no other malloc()/free() here, in DFU at least. // // A new DFU transfer starts, so the buffer is allocated again. // In theory this should succeed since we just free()d a buffer of the // same size. However, this fails because memalign() internally attempts // to allocate much more than "s", yet free(p) above only free()d a // little more than "s". p = memalign(a, s);
Signed-off-by: Stephen Warren swarren@nvidia.com Acked-by: Lukasz Majewski l.majewski@samsung.com Tested-by: Lukasz Majewski l.majewski@samsung.com
Applied to u-boot/master, thanks!
From: Stephen Warren swarren@nvidia.com
When writing to files in a filesystem on MMC, dfu_mmc.c buffers up the entire file content until the end of the transaction, at which point the file is written in one go. This allows writing files larger than the USB transfer size (CONFIG_SYS_DFU_DATA_BUF_SIZE); the maximum written file size is CONFIG_SYS_DFU_MAX_FILE_SIZE (the size of the temporary buffer).
The current file reading code does not do any buffering, and so limits the maximum read file size to the USB transfer size. Enhance the code to do the same kind of buffering as the write path, so the same file size limits apply.
Remove the size checking code from dfu_read() since all read paths now support larger files than the USB transfer buffer.
Signed-off-by: Stephen Warren swarren@nvidia.com --- drivers/dfu/dfu.c | 11 ----------- drivers/dfu/dfu_mmc.c | 27 ++++++++++++++++++++++++++- 2 files changed, 26 insertions(+), 12 deletions(-)
diff --git a/drivers/dfu/dfu.c b/drivers/dfu/dfu.c index d85d3f507a7b..d1e465aa7695 100644 --- a/drivers/dfu/dfu.c +++ b/drivers/dfu/dfu.c @@ -338,17 +338,6 @@ int dfu_read(struct dfu_entity *dfu, void *buf, int size, int blk_seq_num) dfu->r_left = dfu->get_medium_size(dfu); if (dfu->r_left < 0) return dfu->r_left; - switch (dfu->layout) { - case DFU_RAW_ADDR: - case DFU_RAM_ADDR: - break; - default: - if (dfu->r_left > dfu_buf_size) { - printf("%s: File too big for buffer\n", - __func__); - return -EOVERFLOW; - } - }
debug("%s: %s %ld [B]\n", __func__, dfu->name, dfu->r_left);
diff --git a/drivers/dfu/dfu_mmc.c b/drivers/dfu/dfu_mmc.c index 2a780f7b5d31..5a9fb4a6e247 100644 --- a/drivers/dfu/dfu_mmc.c +++ b/drivers/dfu/dfu_mmc.c @@ -18,6 +18,7 @@
static unsigned char *dfu_file_buf; static long dfu_file_buf_len; +static long dfu_file_buf_filled;
static int mmc_access_part(struct dfu_entity *dfu, struct mmc *mmc, int part) { @@ -230,9 +231,12 @@ long dfu_get_medium_size_mmc(struct dfu_entity *dfu) return dfu->data.mmc.lba_size * dfu->data.mmc.lba_blk_size; case DFU_FS_FAT: case DFU_FS_EXT4: + dfu_file_buf_filled = -1; ret = mmc_file_op(DFU_OP_SIZE, dfu, NULL, &len); if (ret < 0) return ret; + if (len > CONFIG_SYS_DFU_MAX_FILE_SIZE) + return -1; return len; default: printf("%s: Layout (%s) not (yet) supported!\n", __func__, @@ -241,6 +245,27 @@ long dfu_get_medium_size_mmc(struct dfu_entity *dfu) } }
+static int mmc_file_unbuffer(struct dfu_entity *dfu, u64 offset, void *buf, + long *len) +{ + int ret; + long file_len; + + if (dfu_file_buf_filled == -1) { + ret = mmc_file_op(DFU_OP_READ, dfu, dfu_file_buf, &file_len); + if (ret < 0) + return ret; + dfu_file_buf_filled = file_len; + } + if (offset + *len > dfu_file_buf_filled) + return -EINVAL; + + /* Add to the current buffer. */ + memcpy(buf, dfu_file_buf + offset, *len); + + return 0; +} + int dfu_read_medium_mmc(struct dfu_entity *dfu, u64 offset, void *buf, long *len) { @@ -252,7 +277,7 @@ int dfu_read_medium_mmc(struct dfu_entity *dfu, u64 offset, void *buf, break; case DFU_FS_FAT: case DFU_FS_EXT4: - ret = mmc_file_op(DFU_OP_READ, dfu, buf, len); + ret = mmc_file_unbuffer(dfu, offset, buf, len); break; default: printf("%s: Layout (%s) not (yet) supported!\n", __func__,
Hi Stephen,
From: Stephen Warren swarren@nvidia.com
When writing to files in a filesystem on MMC, dfu_mmc.c buffers up the entire file content until the end of the transaction, at which point the file is written in one go. This allows writing files larger than the USB transfer size (CONFIG_SYS_DFU_DATA_BUF_SIZE); the maximum written file size is CONFIG_SYS_DFU_MAX_FILE_SIZE (the size of the temporary buffer).
The current file reading code does not do any buffering, and so limits the maximum read file size to the USB transfer size. Enhance the code to do the same kind of buffering as the write path, so the same file size limits apply.
Remove the size checking code from dfu_read() since all read paths now support larger files than the USB transfer buffer.
Signed-off-by: Stephen Warren swarren@nvidia.com
drivers/dfu/dfu.c | 11 ----------- drivers/dfu/dfu_mmc.c | 27 ++++++++++++++++++++++++++- 2 files changed, 26 insertions(+), 12 deletions(-)
diff --git a/drivers/dfu/dfu.c b/drivers/dfu/dfu.c index d85d3f507a7b..d1e465aa7695 100644 --- a/drivers/dfu/dfu.c +++ b/drivers/dfu/dfu.c @@ -338,17 +338,6 @@ int dfu_read(struct dfu_entity *dfu, void *buf, int size, int blk_seq_num) dfu->r_left = dfu->get_medium_size(dfu); if (dfu->r_left < 0) return dfu->r_left;
switch (dfu->layout) {case DFU_RAW_ADDR:case DFU_RAM_ADDR:break;default:if (dfu->r_left > dfu_buf_size) {printf("%s: File too big forbuffer\n",
__func__);return -EOVERFLOW;}}debug("%s: %s %ld [B]\n", __func__, dfu->name,
dfu->r_left); diff --git a/drivers/dfu/dfu_mmc.c b/drivers/dfu/dfu_mmc.c index 2a780f7b5d31..5a9fb4a6e247 100644 --- a/drivers/dfu/dfu_mmc.c +++ b/drivers/dfu/dfu_mmc.c @@ -18,6 +18,7 @@
static unsigned char *dfu_file_buf; static long dfu_file_buf_len; +static long dfu_file_buf_filled;
static int mmc_access_part(struct dfu_entity *dfu, struct mmc *mmc, int part) { @@ -230,9 +231,12 @@ long dfu_get_medium_size_mmc(struct dfu_entity *dfu) return dfu->data.mmc.lba_size * dfu->data.mmc.lba_blk_size; case DFU_FS_FAT: case DFU_FS_EXT4:
dfu_file_buf_filled = -1;if (len > CONFIG_SYS_DFU_MAX_FILE_SIZE)return -1;__func__, @@ -241,6 +245,27 @@ long dfu_get_medium_size_mmc(struct dfu_entity *dfu) } }
+static int mmc_file_unbuffer(struct dfu_entity *dfu, u64 offset, void *buf,
long *len)+{
- int ret;
- long file_len;
- if (dfu_file_buf_filled == -1) {
ret = mmc_file_op(DFU_OP_READ, dfu, dfu_file_buf,&file_len);
if (ret < 0)return ret;dfu_file_buf_filled = file_len;- }
- if (offset + *len > dfu_file_buf_filled)
return -EINVAL;- /* Add to the current buffer. */
- memcpy(buf, dfu_file_buf + offset, *len);
- return 0;
+}
int dfu_read_medium_mmc(struct dfu_entity *dfu, u64 offset, void *buf, long *len) { @@ -252,7 +277,7 @@ int dfu_read_medium_mmc(struct dfu_entity *dfu, u64 offset, void *buf, break; case DFU_FS_FAT: case DFU_FS_EXT4:
ret = mmc_file_op(DFU_OP_READ, dfu, buf, len);
ret = mmc_file_unbuffer(dfu, offset, buf, len);__func__,
Acked-by: Lukasz Majewski l.majewski@samsung.com
Tested-by: Lukasz Majewski l.majewski@samsung.com Test HW: Odroid XU3 - Exynos5433 [DFU Tests]
On Fri, Sep 04, 2015 at 10:03:47PM -0600, Stephen Warren wrote:
From: Stephen Warren swarren@nvidia.com
When writing to files in a filesystem on MMC, dfu_mmc.c buffers up the entire file content until the end of the transaction, at which point the file is written in one go. This allows writing files larger than the USB transfer size (CONFIG_SYS_DFU_DATA_BUF_SIZE); the maximum written file size is CONFIG_SYS_DFU_MAX_FILE_SIZE (the size of the temporary buffer).
The current file reading code does not do any buffering, and so limits the maximum read file size to the USB transfer size. Enhance the code to do the same kind of buffering as the write path, so the same file size limits apply.
Remove the size checking code from dfu_read() since all read paths now support larger files than the USB transfer buffer.
Signed-off-by: Stephen Warren swarren@nvidia.com Acked-by: Lukasz Majewski l.majewski@samsung.com Tested-by: Lukasz Majewski l.majewski@samsung.com
Applied to u-boot/master, thanks!
From: Stephen Warren swarren@nvidia.com
Commit 52a7c98a1772 "tegra-common: increase malloc pool len by dfu mmc file buffer size" updated the definition of CONFIG_SYS_MALLOC_LEN for Tegra to take account of the DFU buffer size. However, this change had no effect, since typical Tegra board config headers don't set the DFU- related defines until after tegra-common.h is included. Fix this by moving the affected conditional code to tegra-common-post.h, which is included last. Also move the definition of SYS_NONCACHED_MEMORY since it's a related and adjacent definition.
Fix the condition to test for the DFU feature, rather than specifically MMC DFU support, so it applies in all cases.
Signed-off-by: Stephen Warren swarren@nvidia.com --- include/configs/tegra-common-post.h | 14 ++++++++++++++ include/configs/tegra-common.h | 14 -------------- 2 files changed, 14 insertions(+), 14 deletions(-)
diff --git a/include/configs/tegra-common-post.h b/include/configs/tegra-common-post.h index e67ff7b95715..594fa456d17f 100644 --- a/include/configs/tegra-common-post.h +++ b/include/configs/tegra-common-post.h @@ -8,6 +8,20 @@ #ifndef __TEGRA_COMMON_POST_H #define __TEGRA_COMMON_POST_H
+/* + * Size of malloc() pool + */ +#ifdef CONFIG_USB_FUNCTION_DFU +#define CONFIG_SYS_MALLOC_LEN ((4 << 20) + \ + CONFIG_SYS_DFU_DATA_BUF_SIZE) +#else +#define CONFIG_SYS_MALLOC_LEN (4 << 20) /* 4MB */ +#endif + +#ifndef CONFIG_ARM64 +#define CONFIG_SYS_NONCACHED_MEMORY (1 << 20) /* 1 MiB */ +#endif + #ifndef CONFIG_SPL_BUILD #define BOOT_TARGET_DEVICES(func) \ func(MMC, mmc, 1) \ diff --git a/include/configs/tegra-common.h b/include/configs/tegra-common.h index 6fe5f2ce6543..b886c6450763 100644 --- a/include/configs/tegra-common.h +++ b/include/configs/tegra-common.h @@ -37,20 +37,6 @@ #define CONFIG_ENV_SIZE 0x2000 /* Total Size Environment */
/* - * Size of malloc() pool - */ -#ifdef CONFIG_DFU_MMC -#define CONFIG_SYS_MALLOC_LEN ((4 << 20) + \ - CONFIG_SYS_DFU_DATA_BUF_SIZE) -#else -#define CONFIG_SYS_MALLOC_LEN (4 << 20) /* 4MB */ -#endif - -#ifndef CONFIG_ARM64 -#define CONFIG_SYS_NONCACHED_MEMORY (1 << 20) /* 1 MiB */ -#endif - -/* * NS16550 Configuration */ #define CONFIG_TEGRA_SERIAL
Stephen,
-----Original Message----- From: Stephen Warren [mailto:swarren@wwwdotorg.org] Sent: Friday, September 04, 2015 9:04 PM To: Tom Rini Cc: u-boot@lists.denx.de; Simon Glass; Tom Warren; Stephen Warren; Lukasz Majewski; Przemyslaw Marczak Subject: [PATCH 07/10] ARM: tegra: fix malloc region sizing
From: Stephen Warren swarren@nvidia.com
Commit 52a7c98a1772 "tegra-common: increase malloc pool len by dfu mmc file buffer size" updated the definition of CONFIG_SYS_MALLOC_LEN for Tegra to take account of the DFU buffer size. However, this change had no effect, since typical Tegra board config headers don't set the DFU- related defines until after tegra-common.h is included. Fix this by moving the affected conditional code to tegra-common-post.h, which is included last. Also move the definition of SYS_NONCACHED_MEMORY since it's a related and adjacent definition.
Fix the condition to test for the DFU feature, rather than specifically MMC DFU support, so it applies in all cases.
Signed-off-by: Stephen Warren swarren@nvidia.com
Do you want me to take these last four in to u-boot-tegra for the pending PR, or do you expect them to go in another way?
Thanks,
Tom -- nvpublic
include/configs/tegra-common-post.h | 14 ++++++++++++++ include/configs/tegra-common.h | 14 -------------- 2 files changed, 14 insertions(+), 14 deletions(-)
diff --git a/include/configs/tegra-common-post.h b/include/configs/tegra- common-post.h index e67ff7b95715..594fa456d17f 100644 --- a/include/configs/tegra-common-post.h +++ b/include/configs/tegra-common-post.h @@ -8,6 +8,20 @@ #ifndef __TEGRA_COMMON_POST_H #define __TEGRA_COMMON_POST_H
+/*
- Size of malloc() pool
- */
+#ifdef CONFIG_USB_FUNCTION_DFU +#define CONFIG_SYS_MALLOC_LEN ((4 << 20) + \
- CONFIG_SYS_DFU_DATA_BUF_SIZE)
+#else +#define CONFIG_SYS_MALLOC_LEN (4 << 20) /* 4MB */ +#endif
+#ifndef CONFIG_ARM64 +#define CONFIG_SYS_NONCACHED_MEMORY (1 << 20) /* 1 MiB */ +#endif
#ifndef CONFIG_SPL_BUILD #define BOOT_TARGET_DEVICES(func) \ func(MMC, mmc, 1) \ diff --git a/include/configs/tegra-common.h b/include/configs/tegra- common.h index 6fe5f2ce6543..b886c6450763 100644 --- a/include/configs/tegra-common.h +++ b/include/configs/tegra-common.h @@ -37,20 +37,6 @@ #define CONFIG_ENV_SIZE 0x2000 /* Total Size Environment */
/*
- Size of malloc() pool
- */
-#ifdef CONFIG_DFU_MMC -#define CONFIG_SYS_MALLOC_LEN ((4 << 20) + \
CONFIG_SYS_DFU_DATA_BUF_SIZE)-#else -#define CONFIG_SYS_MALLOC_LEN (4 << 20) /* 4MB */ -#endif
-#ifndef CONFIG_ARM64 -#define CONFIG_SYS_NONCACHED_MEMORY (1 << 20) /* 1 MiB */ -#endif
-/*
- NS16550 Configuration
*/
#define CONFIG_TEGRA_SERIAL
1.9.1
On 09/08/2015 09:53 AM, Tom Warren wrote:
Stephen,
Stephen Warren wrote at Friday, September 04, 2015 9:04 PM:
From: Stephen Warren swarren@nvidia.com
Commit 52a7c98a1772 "tegra-common: increase malloc pool len by dfu mmc file buffer size" updated the definition of CONFIG_SYS_MALLOC_LEN for Tegra to take account of the DFU buffer size. However, this change had no effect, since typical Tegra board config headers don't set the DFU- related defines until after tegra-common.h is included. Fix this by moving the affected conditional code to tegra-common-post.h, which is included last. Also move the definition of SYS_NONCACHED_MEMORY since it's a related and adjacent definition.
Fix the condition to test for the DFU feature, rather than specifically MMC DFU support, so it applies in all cases.
Signed-off-by: Stephen Warren swarren@nvidia.com
Do you want me to take these last four in to u-boot-tegra for the pending PR, or do you expect them to go in another way?
I believe the 4 "ARM: tegra:" patches can go through the Tegra tree since they're independent from the other patches in the series. Thanks.
On 09/08/2015 02:45 PM, Stephen Warren wrote:
On 09/08/2015 09:53 AM, Tom Warren wrote:
Stephen,
Stephen Warren wrote at Friday, September 04, 2015 9:04 PM:
From: Stephen Warren swarren@nvidia.com
Commit 52a7c98a1772 "tegra-common: increase malloc pool len by dfu mmc file buffer size" updated the definition of CONFIG_SYS_MALLOC_LEN for Tegra to take account of the DFU buffer size. However, this change had no effect, since typical Tegra board config headers don't set the DFU- related defines until after tegra-common.h is included. Fix this by moving the affected conditional code to tegra-common-post.h, which is included last. Also move the definition of SYS_NONCACHED_MEMORY since it's a related and adjacent definition.
Fix the condition to test for the DFU feature, rather than specifically MMC DFU support, so it applies in all cases.
Signed-off-by: Stephen Warren swarren@nvidia.com
Do you want me to take these last four in to u-boot-tegra for the pending PR, or do you expect them to go in another way?
I believe the 4 "ARM: tegra:" patches can go through the Tegra tree since they're independent from the other patches in the series. Thanks.
I note that Lukasz has ack'd all the other patches, so perhaps you can just take the whole series through the Tegra tree? At least the DFU patches since he's maintainer there. Perhaps TomR can ack the ext4 patches since they don't seem to have a maintainer.
On Tue, Sep 08, 2015 at 02:52:06PM -0700, Stephen Warren wrote:
On 09/08/2015 02:45 PM, Stephen Warren wrote:
On 09/08/2015 09:53 AM, Tom Warren wrote:
Stephen,
Stephen Warren wrote at Friday, September 04, 2015 9:04 PM:
From: Stephen Warren swarren@nvidia.com
Commit 52a7c98a1772 "tegra-common: increase malloc pool len by dfu mmc file buffer size" updated the definition of CONFIG_SYS_MALLOC_LEN for Tegra to take account of the DFU buffer size. However, this change had no effect, since typical Tegra board config headers don't set the DFU- related defines until after tegra-common.h is included. Fix this by moving the affected conditional code to tegra-common-post.h, which is included last. Also move the definition of SYS_NONCACHED_MEMORY since it's a related and adjacent definition.
Fix the condition to test for the DFU feature, rather than specifically MMC DFU support, so it applies in all cases.
Signed-off-by: Stephen Warren swarren@nvidia.com
Do you want me to take these last four in to u-boot-tegra for the pending PR, or do you expect them to go in another way?
I believe the 4 "ARM: tegra:" patches can go through the Tegra tree since they're independent from the other patches in the series. Thanks.
I note that Lukasz has ack'd all the other patches, so perhaps you can just take the whole series through the Tegra tree? At least the DFU patches since he's maintainer there. Perhaps TomR can ack the ext4 patches since they don't seem to have a maintainer.
I think I snagged them all in patchwork for myself. Do the Tegra ones depend on anything else?
On 09/08/2015 02:56 PM, Tom Rini wrote:
On Tue, Sep 08, 2015 at 02:52:06PM -0700, Stephen Warren wrote:
On 09/08/2015 02:45 PM, Stephen Warren wrote:
On 09/08/2015 09:53 AM, Tom Warren wrote:
Stephen,
Stephen Warren wrote at Friday, September 04, 2015 9:04 PM:
From: Stephen Warren swarren@nvidia.com
Commit 52a7c98a1772 "tegra-common: increase malloc pool len by dfu mmc file buffer size" updated the definition of CONFIG_SYS_MALLOC_LEN for Tegra to take account of the DFU buffer size. However, this change had no effect, since typical Tegra board config headers don't set the DFU- related defines until after tegra-common.h is included. Fix this by moving the affected conditional code to tegra-common-post.h, which is included last. Also move the definition of SYS_NONCACHED_MEMORY since it's a related and adjacent definition.
Fix the condition to test for the DFU feature, rather than specifically MMC DFU support, so it applies in all cases.
Signed-off-by: Stephen Warren swarren@nvidia.com
Do you want me to take these last four in to u-boot-tegra for the pending PR, or do you expect them to go in another way?
I believe the 4 "ARM: tegra:" patches can go through the Tegra tree since they're independent from the other patches in the series. Thanks.
I note that Lukasz has ack'd all the other patches, so perhaps you can just take the whole series through the Tegra tree? At least the DFU patches since he's maintainer there. Perhaps TomR can ack the ext4 patches since they don't seem to have a maintainer.
I think I snagged them all in patchwork for myself. Do the Tegra ones depend on anything else?
I don't believe the Tegra patches depend on anything to compile or run; they're just all conceptually related to DFU support on Tegra.
Hi Stephen,
On 09/08/2015 02:45 PM, Stephen Warren wrote:
On 09/08/2015 09:53 AM, Tom Warren wrote:
Stephen,
Stephen Warren wrote at Friday, September 04, 2015 9:04 PM:
From: Stephen Warren swarren@nvidia.com
Commit 52a7c98a1772 "tegra-common: increase malloc pool len by dfu mmc file buffer size" updated the definition of CONFIG_SYS_MALLOC_LEN for Tegra to take account of the DFU buffer size. However, this change had no effect, since typical Tegra board config headers don't set the DFU- related defines until after tegra-common.h is included. Fix this by moving the affected conditional code to tegra-common-post.h, which is included last. Also move the definition of SYS_NONCACHED_MEMORY since it's a related and adjacent definition.
Fix the condition to test for the DFU feature, rather than specifically MMC DFU support, so it applies in all cases.
Signed-off-by: Stephen Warren swarren@nvidia.com
Do you want me to take these last four in to u-boot-tegra for the pending PR, or do you expect them to go in another way?
I believe the 4 "ARM: tegra:" patches can go through the Tegra tree since they're independent from the other patches in the series. Thanks.
I note that Lukasz has ack'd all the other patches, so perhaps you can just take the whole series through the Tegra tree? At least the DFU patches since he's maintainer there.
I personally would opt for applying this series to one tree. I've already ack'ed those patches, so those can go to any other tree (or even -dfu one when nobody wants to pick them :-)).
Perhaps TomR can ack the ext4 patches since they don't seem to have a maintainer.
Lack of FS (FAT, EXT4) maintainer is PITTA. I hope that somebody would step up for this position.
-----Original Message----- From: Lukasz Majewski [mailto:l.majewski@samsung.com] Sent: Tuesday, September 08, 2015 11:51 PM To: Stephen Warren Cc: Tom Warren; Tom Rini; Stephen Warren; u-boot@lists.denx.de; Simon Glass; Przemyslaw Marczak Subject: Re: [PATCH 07/10] ARM: tegra: fix malloc region sizing
Hi Stephen,
On 09/08/2015 02:45 PM, Stephen Warren wrote:
On 09/08/2015 09:53 AM, Tom Warren wrote:
Stephen,
Stephen Warren wrote at Friday, September 04, 2015 9:04 PM:
From: Stephen Warren swarren@nvidia.com
Commit 52a7c98a1772 "tegra-common: increase malloc pool len by dfu mmc file buffer size" updated the definition of CONFIG_SYS_MALLOC_LEN for Tegra to take account of the DFU buffer size. However, this change had no effect, since typical Tegra board config headers don't set the DFU- related defines until after tegra-common.h is included. Fix this by moving the affected conditional code to tegra-common-post.h, which is included last. Also move the definition of SYS_NONCACHED_MEMORY since it's a related and adjacent definition.
Fix the condition to test for the DFU feature, rather than specifically MMC DFU support, so it applies in all cases.
Signed-off-by: Stephen Warren swarren@nvidia.com
Do you want me to take these last four in to u-boot-tegra for the pending PR, or do you expect them to go in another way?
I believe the 4 "ARM: tegra:" patches can go through the Tegra tree since they're independent from the other patches in the series. Thanks.
I note that Lukasz has ack'd all the other patches, so perhaps you can just take the whole series through the Tegra tree? At least the DFU patches since he's maintainer there.
I personally would opt for applying this series to one tree. I've already ack'ed those patches, so those can go to any other tree (or even -dfu one when nobody wants to pick them :-)).
I believe TomR has taken over all of these patches in Patchwork, so perhaps he's going to take them in to u-boot/master.
Tom -- nvpublic
Perhaps TomR can ack the ext4 patches since they don't seem to have a maintainer.
Lack of FS (FAT, EXT4) maintainer is PITTA. I hope that somebody would step up for this position.
-- Best regards,
Lukasz Majewski
Samsung R&D Institute Poland (SRPOL) | Linux Platform Group
On Fri, Sep 04, 2015 at 10:03:48PM -0600, Stephen Warren wrote:
From: Stephen Warren swarren@nvidia.com
Commit 52a7c98a1772 "tegra-common: increase malloc pool len by dfu mmc file buffer size" updated the definition of CONFIG_SYS_MALLOC_LEN for Tegra to take account of the DFU buffer size. However, this change had no effect, since typical Tegra board config headers don't set the DFU- related defines until after tegra-common.h is included. Fix this by moving the affected conditional code to tegra-common-post.h, which is included last. Also move the definition of SYS_NONCACHED_MEMORY since it's a related and adjacent definition.
Fix the condition to test for the DFU feature, rather than specifically MMC DFU support, so it applies in all cases.
Signed-off-by: Stephen Warren swarren@nvidia.com
Applied to u-boot/master, thanks!
From: Stephen Warren swarren@nvidia.com
CONFIG_SYS_DFU_DATA_BUF_SIZE defines the size of chunks transferred across USB. This doesn't need to be particularly large, since it doesn't limit the overall transfer size.
CONFIG_SYS_DFU_MAX_FILE_SIZE is used to buffer an entire file before writing it to a filesystem. This define limits the maximum file size that may be transferred. Bump this up to 32MiB in order to support large uncompressed kernel images.
Both of these buffers are dynamically allocated, and so the size of both needs to be taken into account when calculating the required malloc region size.
Signed-off-by: Stephen Warren swarren@nvidia.com --- include/configs/tegra-common-post.h | 5 +++-- include/configs/tegra-common-usb-gadget.h | 3 ++- 2 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/include/configs/tegra-common-post.h b/include/configs/tegra-common-post.h index 594fa456d17f..651c4c493536 100644 --- a/include/configs/tegra-common-post.h +++ b/include/configs/tegra-common-post.h @@ -12,8 +12,9 @@ * Size of malloc() pool */ #ifdef CONFIG_USB_FUNCTION_DFU -#define CONFIG_SYS_MALLOC_LEN ((4 << 20) + \ - CONFIG_SYS_DFU_DATA_BUF_SIZE) +#define CONFIG_SYS_MALLOC_LEN (SZ_4M + \ + CONFIG_SYS_DFU_DATA_BUF_SIZE + \ + CONFIG_SYS_DFU_MAX_FILE_SIZE) #else #define CONFIG_SYS_MALLOC_LEN (4 << 20) /* 4MB */ #endif diff --git a/include/configs/tegra-common-usb-gadget.h b/include/configs/tegra-common-usb-gadget.h index e51da3f40571..b1a14cbe73b5 100644 --- a/include/configs/tegra-common-usb-gadget.h +++ b/include/configs/tegra-common-usb-gadget.h @@ -30,7 +30,8 @@ #define CONFIG_CMD_USB_MASS_STORAGE /* DFU protocol */ #define CONFIG_USB_FUNCTION_DFU -#define CONFIG_SYS_DFU_DATA_BUF_SIZE (1 * 1024 * 1024) +#define CONFIG_SYS_DFU_DATA_BUF_SIZE SZ_1M +#define CONFIG_SYS_DFU_MAX_FILE_SIZE SZ_32M #define CONFIG_CMD_DFU #ifdef CONFIG_MMC #define CONFIG_DFU_MMC
On Fri, Sep 04, 2015 at 10:03:49PM -0600, Stephen Warren wrote:
From: Stephen Warren swarren@nvidia.com
CONFIG_SYS_DFU_DATA_BUF_SIZE defines the size of chunks transferred across USB. This doesn't need to be particularly large, since it doesn't limit the overall transfer size.
CONFIG_SYS_DFU_MAX_FILE_SIZE is used to buffer an entire file before writing it to a filesystem. This define limits the maximum file size that may be transferred. Bump this up to 32MiB in order to support large uncompressed kernel images.
Both of these buffers are dynamically allocated, and so the size of both needs to be taken into account when calculating the required malloc region size.
Signed-off-by: Stephen Warren swarren@nvidia.com
Applied to u-boot/master, thanks!
From: Stephen Warren swarren@nvidia.com
Writing to files is a useful feature in general, so enable it everywhere. The primary purpose is to make DFU useful on filesystems in addition to raw devices.
Signed-off-by: Stephen Warren swarren@nvidia.com --- include/configs/tegra-common.h | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/include/configs/tegra-common.h b/include/configs/tegra-common.h index b886c6450763..1c469d092e8c 100644 --- a/include/configs/tegra-common.h +++ b/include/configs/tegra-common.h @@ -140,6 +140,8 @@
#ifndef CONFIG_SPL_BUILD #include <config_distro_defaults.h> +#define CONFIG_CMD_EXT4_WRITE +#define CONFIG_FAT_WRITE #endif
#endif /* _TEGRA_COMMON_H_ */
On Fri, Sep 04, 2015 at 10:03:50PM -0600, Stephen Warren wrote:
From: Stephen Warren swarren@nvidia.com
Writing to files is a useful feature in general, so enable it everywhere. The primary purpose is to make DFU useful on filesystems in addition to raw devices.
Signed-off-by: Stephen Warren swarren@nvidia.com
Applied to u-boot/master, thanks!
From: Stephen Warren swarren@nvidia.com
This allows transferring data directly to/from RAM. For example, one could create a boot script that starts DFU on a RAM region, then once DFU exits (which is under the control of the attached USB host, via a USB bus reset), uses the code/data that was received over DFU.
Signed-off-by: Stephen Warren swarren@nvidia.com --- include/configs/tegra-common-usb-gadget.h | 1 + 1 file changed, 1 insertion(+)
diff --git a/include/configs/tegra-common-usb-gadget.h b/include/configs/tegra-common-usb-gadget.h index b1a14cbe73b5..f6e1d5c4db2d 100644 --- a/include/configs/tegra-common-usb-gadget.h +++ b/include/configs/tegra-common-usb-gadget.h @@ -39,6 +39,7 @@ #ifdef CONFIG_SPI_FLASH #define CONFIG_DFU_SF #endif +#define CONFIG_DFU_RAM #endif
#endif /* _TEGRA_COMMON_USB_GADGET_H_ */
On Fri, Sep 04, 2015 at 10:03:51PM -0600, Stephen Warren wrote:
From: Stephen Warren swarren@nvidia.com
This allows transferring data directly to/from RAM. For example, one could create a boot script that starts DFU on a RAM region, then once DFU exits (which is under the control of the attached USB host, via a USB bus reset), uses the code/data that was received over DFU.
Signed-off-by: Stephen Warren swarren@nvidia.com
Applied to u-boot/master, thanks!
Hi Stephen,
From: Stephen Warren swarren@nvidia.com
By the time g_dnl_unbind() is run, cdev->config has been set to NULL, so the free() there does nothing, and the config struct is leaked. Equally, struct usb_gadget contains a linked list of config structs, so the code should iterate over them all and free each one, rather than freeing one particular config struct.
composite_unbind() already iterates over the list of config structs, and unlinks each from the linked list. Fix this loop to free() each struct as it's unlinked and otherwise forgotten.
Signed-off-by: Stephen Warren swarren@nvidia.com
drivers/usb/gadget/composite.c | 1 + drivers/usb/gadget/g_dnl.c | 2 -- 2 files changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c index d96296cd73b1..a13b21d0a0f2 100644 --- a/drivers/usb/gadget/composite.c +++ b/drivers/usb/gadget/composite.c @@ -948,6 +948,7 @@ static void composite_unbind(struct usb_gadget *gadget) debug("unbind config '%s'/%p\n", c->label, c); c->unbind(c); }
free(c);diff --git a/drivers/usb/gadget/g_dnl.c b/drivers/usb/gadget/g_dnl.c index ad89a0d2e670..2fa6da4091e1 100644 --- a/drivers/usb/gadget/g_dnl.c +++ b/drivers/usb/gadget/g_dnl.c @@ -93,8 +93,6 @@ static int g_dnl_unbind(struct usb_composite_dev *cdev) { struct usb_gadget *gadget = cdev->gadget;
- free(cdev->config);
- cdev->config = NULL; debug("%s: calling usb_gadget_disconnect for " "controller '%s'\n", __func__, gadget->name); usb_gadget_disconnect(gadget);
Acked-by: Lukasz Majewski l.majewski@samsung.com
Tested-by: Lukasz Majewski l.majewski@samsung.com Test HW: Odroid XU3 - Exynos5433.
On 09/08/2015 05:00 AM, Lukasz Majewski wrote:
Hi Stephen,
From: Stephen Warren swarren@nvidia.com
By the time g_dnl_unbind() is run, cdev->config has been set to NULL, so the free() there does nothing, and the config struct is leaked. Equally, struct usb_gadget contains a linked list of config structs, so the code should iterate over them all and free each one, rather than freeing one particular config struct.
composite_unbind() already iterates over the list of config structs, and unlinks each from the linked list. Fix this loop to free() each struct as it's unlinked and otherwise forgotten.
...
Acked-by: Lukasz Majewski l.majewski@samsung.com
Tested-by: Lukasz Majewski l.majewski@samsung.com Test HW: Odroid XU3 - Exynos5433.
Thanks for all the tests/acks.
On Fri, Sep 04, 2015 at 10:03:42PM -0600, Stephen Warren wrote:
From: Stephen Warren swarren@nvidia.com
By the time g_dnl_unbind() is run, cdev->config has been set to NULL, so the free() there does nothing, and the config struct is leaked. Equally, struct usb_gadget contains a linked list of config structs, so the code should iterate over them all and free each one, rather than freeing one particular config struct.
composite_unbind() already iterates over the list of config structs, and unlinks each from the linked list. Fix this loop to free() each struct as it's unlinked and otherwise forgotten.
Signed-off-by: Stephen Warren swarren@nvidia.com Acked-by: Lukasz Majewski l.majewski@samsung.com Tested-by: Lukasz Majewski l.majewski@samsung.com
Applied to u-boot/master, thanks!
participants (5)
-
Lukasz Majewski -
Stephen Warren -
Stephen Warren -
Tom Rini -
Tom Warren