| Message ID | 20240916152025.75789-2-sughosh.ganu@linaro.org |
|---|---|
| State | Accepted |
| Commit | af45c84871e4c43a021bb12c71f70e8ded956068 |
| 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 a tftp 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/tftp.c | 19 +------------------ > 1 file changed, 1 insertion(+), 18 deletions(-) > > diff --git a/net/tftp.c b/net/tftp.c > index b5d227d8bc..d6744bc24e 100644 > --- a/net/tftp.c > +++ b/net/tftp.c > @@ -82,7 +82,6 @@ static ulong tftp_block_wrap; > static ulong tftp_block_wrap_offset; > static int tftp_state; > static ulong tftp_load_addr; > -static ulong tftp_load_size; > #ifdef CONFIG_TFTP_TSIZE > /* The file size reported by the server */ > static int tftp_tsize; > @@ -159,13 +158,8 @@ static inline int store_block(int block, uchar *src, unsigned int len) > void *ptr; > > if (CONFIG_IS_ENABLED(LMB)) { > - ulong end_addr = tftp_load_addr + tftp_load_size; > - > - if (!end_addr) > - end_addr = ULONG_MAX; > - > if (store_addr < tftp_load_addr || > - store_addr + len > end_addr) { > + lmb_read_check(store_addr, len)) { > puts("\nTFTP error: "); > puts("trying to overwrite reserved memory...\n"); > return -1; > @@ -712,19 +706,8 @@ static void tftp_timeout_handler(void) > } > } > > -/* Initialize tftp_load_addr and tftp_load_size from image_load_addr and lmb */ > static int tftp_init_load_addr(void) > { > - if (CONFIG_IS_ENABLED(LMB)) { > - phys_size_t max_size; > - > - max_size = lmb_get_free_size(image_load_addr); > - if (!max_size) > - return -1; > - > - tftp_load_size = max_size; > - } > - > tftp_load_addr = image_load_addr; > return 0; > }
diff --git a/net/tftp.c b/net/tftp.c index b5d227d8bc..d6744bc24e 100644 --- a/net/tftp.c +++ b/net/tftp.c @@ -82,7 +82,6 @@ static ulong tftp_block_wrap; static ulong tftp_block_wrap_offset; static int tftp_state; static ulong tftp_load_addr; -static ulong tftp_load_size; #ifdef CONFIG_TFTP_TSIZE /* The file size reported by the server */ static int tftp_tsize; @@ -159,13 +158,8 @@ static inline int store_block(int block, uchar *src, unsigned int len) void *ptr; if (CONFIG_IS_ENABLED(LMB)) { - ulong end_addr = tftp_load_addr + tftp_load_size; - - if (!end_addr) - end_addr = ULONG_MAX; - if (store_addr < tftp_load_addr || - store_addr + len > end_addr) { + lmb_read_check(store_addr, len)) { puts("\nTFTP error: "); puts("trying to overwrite reserved memory...\n"); return -1; @@ -712,19 +706,8 @@ static void tftp_timeout_handler(void) } } -/* Initialize tftp_load_addr and tftp_load_size from image_load_addr and lmb */ static int tftp_init_load_addr(void) { - if (CONFIG_IS_ENABLED(LMB)) { - phys_size_t max_size; - - max_size = lmb_get_free_size(image_load_addr); - if (!max_size) - return -1; - - tftp_load_size = max_size; - } - tftp_load_addr = image_load_addr; return 0; }
Use the lmb_read_check() function to verify if it is safe to use a region of memory to load data from a tftp 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/tftp.c | 19 +------------------ 1 file changed, 1 insertion(+), 18 deletions(-)