| Message ID | 20240916152025.75789-3-sughosh.ganu@linaro.org |
|---|---|
| State | Accepted |
| Commit | 51ebd514ec78617482ff92e4577042e7a2d4b8f1 |
| Headers | show |
| Series | lmb: rework logic to validate load address for network commands | expand |
On 16/09/24 20:50, Sughosh Ganu wrote: > Use the lmb_read_check() function to verify if it is safe to use a > region of memory to load data from the wget command. The current logic > checks the amount of free memory available, starting from the 'load > address'. This call fails if the same region of memory has been used > earlier. This used to work earlier as the LMB memory map had a local > scope and was not persistent. Fix this issue by using the > lmb_read_check() call instead which only returns an error in case the > memory region has been marked for not allowing re-use. > > Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org> > --- > > Note: To be applied on next, on top of > https://patchwork.ozlabs.org/project/uboot/patch/20240913073251.2286529-2-prasad.kummari@amd.com/ > Tested-by: Vaishnav Achath <vaishnav.a@ti.com> > net/wget.c | 36 +----------------------------------- > 1 file changed, 1 insertion(+), 35 deletions(-) > > diff --git a/net/wget.c b/net/wget.c > index 4a168641c6..a88c31f236 100644 > --- a/net/wget.c > +++ b/net/wget.c > @@ -64,26 +64,6 @@ static unsigned int retry_tcp_ack_num; /* TCP retry acknowledge number*/ > static unsigned int retry_tcp_seq_num; /* TCP retry sequence number */ > static int retry_len; /* TCP retry length */ > > -static ulong wget_load_size; > - > -/** > - * wget_init_max_size() - initialize maximum load size > - * > - * Return: 0 if success, -1 if fails > - */ > -static int wget_init_load_size(void) > -{ > - phys_size_t max_size; > - > - max_size = lmb_get_free_size(image_load_addr); > - if (!max_size) > - return -1; > - > - wget_load_size = max_size; > - > - return 0; > -} > - > /** > * store_block() - store block in memory > * @src: source of data > @@ -97,13 +77,8 @@ static inline int store_block(uchar *src, unsigned int offset, unsigned int len) > uchar *ptr; > > if (CONFIG_IS_ENABLED(LMB)) { > - ulong end_addr = image_load_addr + wget_load_size; > - > - if (!end_addr) > - end_addr = ULONG_MAX; > - > if (store_addr < image_load_addr || > - store_addr + len > end_addr) { > + lmb_read_check(store_addr, len)) { > printf("\nwget error: "); > printf("trying to overwrite reserved memory...\n"); > return -1; > @@ -493,15 +468,6 @@ void wget_start(void) > debug_cond(DEBUG_WGET, > "\nwget:Load address: 0x%lx\nLoading: *\b", image_load_addr); > > - if (CONFIG_IS_ENABLED(LMB)) { > - if (wget_init_load_size()) { > - printf("\nwget error: "); > - printf("trying to overwrite reserved memory...\n"); > - net_set_state(NETLOOP_FAIL); > - return; > - } > - } > - > net_set_timeout_handler(wget_timeout, wget_timeout_handler); > tcp_set_tcp_handler(wget_handler); >
On 16/09/2024 17:20, Sughosh Ganu wrote: > Use the lmb_read_check() function to verify if it is safe to use a > region of memory to load data from the wget command. The current logic > checks the amount of free memory available, starting from the 'load > address'. This call fails if the same region of memory has been used > earlier. This used to work earlier as the LMB memory map had a local > scope and was not persistent. Fix this issue by using the > lmb_read_check() call instead which only returns an error in case the > memory region has been marked for not allowing re-use. > > Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org> This fixes Qualcomm platforms which use LMB to allocate $loaddaddr and friends. Reviewed-by: Caleb Connolly <caleb.connolly@linaro.org> > --- > > Note: To be applied on next, on top of > https://patchwork.ozlabs.org/project/uboot/patch/20240913073251.2286529-2-prasad.kummari@amd.com/ > > net/wget.c | 36 +----------------------------------- > 1 file changed, 1 insertion(+), 35 deletions(-) > > diff --git a/net/wget.c b/net/wget.c > index 4a168641c6..a88c31f236 100644 > --- a/net/wget.c > +++ b/net/wget.c > @@ -64,26 +64,6 @@ static unsigned int retry_tcp_ack_num; /* TCP retry acknowledge number*/ > static unsigned int retry_tcp_seq_num; /* TCP retry sequence number */ > static int retry_len; /* TCP retry length */ > > -static ulong wget_load_size; > - > -/** > - * wget_init_max_size() - initialize maximum load size > - * > - * Return: 0 if success, -1 if fails > - */ > -static int wget_init_load_size(void) > -{ > - phys_size_t max_size; > - > - max_size = lmb_get_free_size(image_load_addr); > - if (!max_size) > - return -1; > - > - wget_load_size = max_size; > - > - return 0; > -} > - > /** > * store_block() - store block in memory > * @src: source of data > @@ -97,13 +77,8 @@ static inline int store_block(uchar *src, unsigned int offset, unsigned int len) > uchar *ptr; > > if (CONFIG_IS_ENABLED(LMB)) { > - ulong end_addr = image_load_addr + wget_load_size; > - > - if (!end_addr) > - end_addr = ULONG_MAX; > - > if (store_addr < image_load_addr || > - store_addr + len > end_addr) { > + lmb_read_check(store_addr, len)) { > printf("\nwget error: "); > printf("trying to overwrite reserved memory...\n"); > return -1; > @@ -493,15 +468,6 @@ void wget_start(void) > debug_cond(DEBUG_WGET, > "\nwget:Load address: 0x%lx\nLoading: *\b", image_load_addr); > > - if (CONFIG_IS_ENABLED(LMB)) { > - if (wget_init_load_size()) { > - printf("\nwget error: "); > - printf("trying to overwrite reserved memory...\n"); > - net_set_state(NETLOOP_FAIL); > - return; > - } > - } > - > net_set_timeout_handler(wget_timeout, wget_timeout_handler); > tcp_set_tcp_handler(wget_handler); >
diff --git a/net/wget.c b/net/wget.c index 4a168641c6..a88c31f236 100644 --- a/net/wget.c +++ b/net/wget.c @@ -64,26 +64,6 @@ static unsigned int retry_tcp_ack_num; /* TCP retry acknowledge number*/ static unsigned int retry_tcp_seq_num; /* TCP retry sequence number */ static int retry_len; /* TCP retry length */ -static ulong wget_load_size; - -/** - * wget_init_max_size() - initialize maximum load size - * - * Return: 0 if success, -1 if fails - */ -static int wget_init_load_size(void) -{ - phys_size_t max_size; - - max_size = lmb_get_free_size(image_load_addr); - if (!max_size) - return -1; - - wget_load_size = max_size; - - return 0; -} - /** * store_block() - store block in memory * @src: source of data @@ -97,13 +77,8 @@ static inline int store_block(uchar *src, unsigned int offset, unsigned int len) uchar *ptr; if (CONFIG_IS_ENABLED(LMB)) { - ulong end_addr = image_load_addr + wget_load_size; - - if (!end_addr) - end_addr = ULONG_MAX; - if (store_addr < image_load_addr || - store_addr + len > end_addr) { + lmb_read_check(store_addr, len)) { printf("\nwget error: "); printf("trying to overwrite reserved memory...\n"); return -1; @@ -493,15 +468,6 @@ void wget_start(void) debug_cond(DEBUG_WGET, "\nwget:Load address: 0x%lx\nLoading: *\b", image_load_addr); - if (CONFIG_IS_ENABLED(LMB)) { - if (wget_init_load_size()) { - printf("\nwget error: "); - printf("trying to overwrite reserved memory...\n"); - net_set_state(NETLOOP_FAIL); - return; - } - } - net_set_timeout_handler(wget_timeout, wget_timeout_handler); tcp_set_tcp_handler(wget_handler);
Use the lmb_read_check() function to verify if it is safe to use a region of memory to load data from the wget command. The current logic checks the amount of free memory available, starting from the 'load address'. This call fails if the same region of memory has been used earlier. This used to work earlier as the LMB memory map had a local scope and was not persistent. Fix this issue by using the lmb_read_check() call instead which only returns an error in case the memory region has been marked for not allowing re-use. Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org> --- Note: To be applied on next, on top of https://patchwork.ozlabs.org/project/uboot/patch/20240913073251.2286529-2-prasad.kummari@amd.com/ net/wget.c | 36 +----------------------------------- 1 file changed, 1 insertion(+), 35 deletions(-)