linux-kernel - [POSSIBLE BUG] Unreachable code or possible dereferencing of NULL pointer

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [day] [month] [year] [list]

Message-ID: <f483778df438c24ac57d660785c71402a1ba2d2c.camel@ispras.ru>
Date:   Tue, 05 Jul 2022 18:52:45 +0300
From:   Subkhankulov Rustam <subkhankulov@...ras.ru>
To:     David Airlie <airlied@...ux.ie>
Cc:     Daniel Vetter <daniel@...ll.ch>, dri-devel@...ts.freedesktop.org,
        linux-kernel@...r.kernel.org,
        Alexey Khoroshilov <khoroshilov@...ras.ru>
Subject: [POSSIBLE BUG] Unreachable code or possible dereferencing of NULL
 pointer

Version: 5.19-rc5

In function 'via_do_init_map' (drivers/gpu/drm/via/via_map.c: 54)
'drm_legacy_findmap' can return NULL pointer. If that happens,it calls
'via_do_cleanup_map' (drivers/gpu/drm/via/via_map.c: 58).

---------------------------------------------------------------------
54    dev_priv->mmio = drm_legacy_findmap(dev, init->mmio_offset);
55    if (!dev_priv->mmio) {
56        DRM_ERROR("could not find mmio region!\n");
57        dev->dev_private = (void *)dev_priv;
58        via_do_cleanup_map(dev);
59        return -EINVAL;
60    }
---------------------------------------------------------------------

'via_do_cleanup' functions calls
'via_dma_cleanup'(drivers/gpu/drm/via/via_map.c: 78).

---------------------------------------------------------------------
76    int via_do_cleanup_map(struct drm_device *dev)
77    {
78        via_dma_cleanup(dev);
79
80        return 0;
81    }
---------------------------------------------------------------------

In 'via_dma_cleanup' there is another conditional construction
(drivers/gpu/drm/via/via_dma.c: 168). 

---------------------------------------------------------------------
168   if (dev_priv->ring.virtual_start) {
169       via_cmdbuf_reset(dev_priv);
170
171       drm_legacy_ioremapfree(&dev_priv->ring.map, dev);
172       dev_priv->ring.virtual_start = NULL;
173   }
---------------------------------------------------------------------

It seems like there are two possible ways: 

1) dev_priv->ring.virtual_start != 0. 

In that case function call chain happens: 'via_cmdbuf_reset',
'via_cmdbuf_flush', 'via_hook_segment' and 'via_read'
(drivers/gpu/drm/via/via_drv.h: 124).
In 'via_read' dereferencing of "dev_priv->mmio" happens, which is NULL.

---------------------------------------------------------------------
124    static inline u32 via_read(struct drm_via_private *dev_priv, u32
reg)
125    {
126        return readl((void __iomem *)(dev_priv->mmio->handle +
reg));
127    }
---------------------------------------------------------------------

2) dev_priv->ring.virtual_start == 0.
Then all function calls located inside conditional construction
(drivers/gpu/drm/via/via_dma.c: 168) do not happen.

Thus, if dev_priv->mmio == NULL, call of 'via_do_cleanup_map'
(drivers/gpu/drm/via/via_map.c: 58) may result in either an error or
nothing at all.
Should we remove call to via_do_cleanup_map(dev) or should we somehow
avoid NULL pointer dereference in 'via_read'?

Found by Linux Verification Center (linuxtesting.org) with SVACE.

regards,
Rustam Subkhankulov