Re: [PATCH] Mini-OS: fix 9pfs frontend error path

[Samuel Thibault <samuel.thibault@xxxxxxxxxxxx>]

Juergen Gross, le mar. 06 févr. 2024 07:17:21 +0100, a ecrit:
> The early error exit in p9_stat() returns without zeroing the p9_stat
> buffer, resulting in free() being called with an uninitialized pointer.
> 
> Fix that by doing the zeroing first.

This is not coherent with the usual conventions: when a function fails,
it is supposed not to have done anything, and thus the caller shouldn't
have to clean anything.

I.e. i'd rather see the free_stat() call be put after the check for
an error returned by p9_stat.

> Reported-by: Julien Grall <julien@xxxxxxx>
> Fixes: 2d1dfccd3aa3 ("Mini-OS: add read and write support to 9pfsfront")
> Signed-off-by: Juergen Gross <jgross@xxxxxxxx>
> ---
>  9pfront.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/9pfront.c b/9pfront.c
> index 315089bc..33eaadce 100644
> --- a/9pfront.c
> +++ b/9pfront.c
> @@ -716,10 +716,11 @@ static int p9_stat(struct dev_9pfs *dev, uint32_t fid, 
> struct p9_stat *stat)
>      uint16_t total;
>      int ret;
>  
> +    memset(stat, 0, sizeof(*stat));
> +
>      if ( !req )
>          return EAGAIN;
>  
> -    memset(stat, 0, sizeof(*stat));
>      req->cmd = P9_CMD_STAT;
>      send_9p(dev, req, "U", fid);
>      rcv_9p(dev, req, "uuuUQUUULSSSSSUUU", &total, &stat->size, &stat->type,
> -- 
> 2.35.3