Re: [PATCH RESEND 2/3] vhost: fix a null pointer reference of vhost_log

[Michael S. Tsirkin]

On Mon, Feb 24, 2020 at 02:42:18PM +0800, Longpeng(Mike) wrote:
> From: Longpeng <address@hidden>
> 
> vhost_log_alloc() may fails and returned pointer of log is null.
> However there're two places derefernce the return pointer without
> check.
> 
> Signed-off-by: Longpeng <address@hidden>
> ---
>  hw/virtio/vhost.c | 19 +++++++++++++++++--
>  1 file changed, 17 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
> index 9edfadc..c7ad6e5 100644
> --- a/hw/virtio/vhost.c
> +++ b/hw/virtio/vhost.c
> @@ -219,6 +219,10 @@ static struct vhost_log *vhost_log_get(uint64_t size, 
> bool share)
>  
>      if (!log || log->size != size) {
>          log = vhost_log_alloc(size, share);
> +        if (!log) {
> +            return NULL;
> +        }
> +
>          if (share) {
>              vhost_log_shm = log;
>          } else {
> @@ -270,10 +274,17 @@ static bool vhost_dev_log_is_shared(struct vhost_dev 
> *dev)
>  
>  static inline void vhost_dev_log_resize(struct vhost_dev *dev, uint64_t size)
>  {
> -    struct vhost_log *log = vhost_log_get(size, 
> vhost_dev_log_is_shared(dev));
> -    uint64_t log_base = (uintptr_t)log->log;
> +    struct vhost_log *log;
> +    uint64_t log_base;
>      int r;
>  
> +    log = vhost_log_get(size, vhost_dev_log_is_shared(dev));
> +    if (!log) {
> +        return;
> +    }
> +

I'm not sure silently failing like this is safe. Callers assume
log can be resized. What can be done? I suspect not much
beside exiting ...
Speaking of which, lots of other failures in log resizing
path seem to be silently ignored.
I guess we should propagate them, and fix callers to check
the return code?