[PATCH 1/2] tftp: rework the logic to validate the load address

[Vaishnav Achath]

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 at 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 at 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;
>   }