* [POSSIBLE BUG] iommu/io-pgtable-arm: possible dereferencing of NULL pointer
@ 2022-07-18 9:20 ` Subkhankulov Rustam
0 siblings, 0 replies; 8+ messages in thread
From: Subkhankulov Rustam @ 2022-07-18 9:20 UTC (permalink / raw)
To: Will Deacon
Cc: Robin Murphy, Joerg Roedel, linux-arm-kernel, iommu,
linux-kernel, Alexey Khoroshilov, ldv-project
Version: 5-19-rc6
In function '__arm_lpae_alloc_pages' pointer 'dev' is compared with
NULL at [drivers/iommu/io-pgtable-arm.c: 203]. This means that the
pointer can be NULL.
-----------------------------------------------------------------------
203 p = alloc_pages_node(dev ? dev_to_node(dev) : NUMA_NO_NODE,
204 gfp | __GFP_ZERO, order);
-----------------------------------------------------------------------
Then, if cfg->coherent_walk == 0 at [drivers/iommu/io-pgtable-arm.c:
209], function 'dma_map_single', which is defined as
'dma_map_single_attrs', is called and pointer dev is passed as
first parameter.
-----------------------------------------------------------------------
209 if (!cfg->coherent_walk) {
208 dma = dma_map_single(dev, pages, size, DMA_TO_DEVICE);
-----------------------------------------------------------------------
Therefore, pointer 'dev' passed to function 'dev_driver_string'
in macro 'dev_WARN_ONCE' at [include/linux/dma-mapping.h: 326],
where it is dereferenced at [drivers/base/core.c: 2091].
-----------------------------------------------------------------------
2083 const char *dev_driver_string(const struct device *dev)
2084 {
2085 struct device_driver *drv;
2086
---
2091 drv = READ_ONCE(dev->driver);
-----------------------------------------------------------------------
Thus, if it is possible that 'dev' is null at the same time
that flag 'coherent_walk' is 0, then NULL pointer will be
dereferenced.
Should we somehow avoid NULL pointer dereference or is this
situation impossible and we should remove comparison with NULL?
Found by Linux Verification Center (linuxtesting.org) with SVACE.
regards,
Rustam Subkhankulov
^ permalink raw reply [flat|nested] 8+ messages in thread
* [POSSIBLE BUG] iommu/io-pgtable-arm: possible dereferencing of NULL pointer
@ 2022-07-18 9:20 ` Subkhankulov Rustam
0 siblings, 0 replies; 8+ messages in thread
From: Subkhankulov Rustam @ 2022-07-18 9:20 UTC (permalink / raw)
To: Will Deacon
Cc: Robin Murphy, Joerg Roedel, linux-arm-kernel, iommu,
linux-kernel, Alexey Khoroshilov, ldv-project
Version: 5-19-rc6
In function '__arm_lpae_alloc_pages' pointer 'dev' is compared with
NULL at [drivers/iommu/io-pgtable-arm.c: 203]. This means that the
pointer can be NULL.
-----------------------------------------------------------------------
203 p = alloc_pages_node(dev ? dev_to_node(dev) : NUMA_NO_NODE,
204 gfp | __GFP_ZERO, order);
-----------------------------------------------------------------------
Then, if cfg->coherent_walk == 0 at [drivers/iommu/io-pgtable-arm.c:
209], function 'dma_map_single', which is defined as
'dma_map_single_attrs', is called and pointer dev is passed as
first parameter.
-----------------------------------------------------------------------
209 if (!cfg->coherent_walk) {
208 dma = dma_map_single(dev, pages, size, DMA_TO_DEVICE);
-----------------------------------------------------------------------
Therefore, pointer 'dev' passed to function 'dev_driver_string'
in macro 'dev_WARN_ONCE' at [include/linux/dma-mapping.h: 326],
where it is dereferenced at [drivers/base/core.c: 2091].
-----------------------------------------------------------------------
2083 const char *dev_driver_string(const struct device *dev)
2084 {
2085 struct device_driver *drv;
2086
---
2091 drv = READ_ONCE(dev->driver);
-----------------------------------------------------------------------
Thus, if it is possible that 'dev' is null at the same time
that flag 'coherent_walk' is 0, then NULL pointer will be
dereferenced.
Should we somehow avoid NULL pointer dereference or is this
situation impossible and we should remove comparison with NULL?
Found by Linux Verification Center (linuxtesting.org) with SVACE.
regards,
Rustam Subkhankulov
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [POSSIBLE BUG] iommu/io-pgtable-arm: possible dereferencing of NULL pointer
2022-07-18 9:20 ` Subkhankulov Rustam
@ 2022-07-19 17:36 ` Will Deacon
-1 siblings, 0 replies; 8+ messages in thread
From: Will Deacon @ 2022-07-19 17:36 UTC (permalink / raw)
To: Subkhankulov Rustam
Cc: Robin Murphy, Joerg Roedel, linux-arm-kernel, iommu,
linux-kernel, Alexey Khoroshilov, ldv-project
On Mon, Jul 18, 2022 at 12:20:06PM +0300, Subkhankulov Rustam wrote:
> Version: 5-19-rc6
>
> In function '__arm_lpae_alloc_pages' pointer 'dev' is compared with
> NULL at [drivers/iommu/io-pgtable-arm.c: 203]. This means that the
> pointer can be NULL.
>
> -----------------------------------------------------------------------
> 203 p = alloc_pages_node(dev ? dev_to_node(dev) : NUMA_NO_NODE,
> 204 gfp | __GFP_ZERO, order);
> -----------------------------------------------------------------------
>
> Then, if cfg->coherent_walk == 0 at [drivers/iommu/io-pgtable-arm.c:
> 209], function 'dma_map_single', which is defined as
> 'dma_map_single_attrs', is called and pointer dev is passed as
> first parameter.
>
> -----------------------------------------------------------------------
> 209 if (!cfg->coherent_walk) {
> 208 dma = dma_map_single(dev, pages, size, DMA_TO_DEVICE);
> -----------------------------------------------------------------------
>
> Therefore, pointer 'dev' passed to function 'dev_driver_string'
> in macro 'dev_WARN_ONCE' at [include/linux/dma-mapping.h: 326],
> where it is dereferenced at [drivers/base/core.c: 2091].
>
> -----------------------------------------------------------------------
> 2083 const char *dev_driver_string(const struct device *dev)
> 2084 {
> 2085 struct device_driver *drv;
> 2086
> ---
> 2091 drv = READ_ONCE(dev->driver);
> -----------------------------------------------------------------------
>
> Thus, if it is possible that 'dev' is null at the same time
> that flag 'coherent_walk' is 0, then NULL pointer will be
> dereferenced.
>
> Should we somehow avoid NULL pointer dereference or is this
> situation impossible and we should remove comparison with NULL?
I think 'dev' is only null in the case of the selftest initcall
(see arm_lpae_do_selftests()), and 'coherent_walk' is always true there.
Will
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [POSSIBLE BUG] iommu/io-pgtable-arm: possible dereferencing of NULL pointer
@ 2022-07-19 17:36 ` Will Deacon
0 siblings, 0 replies; 8+ messages in thread
From: Will Deacon @ 2022-07-19 17:36 UTC (permalink / raw)
To: Subkhankulov Rustam
Cc: Robin Murphy, Joerg Roedel, linux-arm-kernel, iommu,
linux-kernel, Alexey Khoroshilov, ldv-project
On Mon, Jul 18, 2022 at 12:20:06PM +0300, Subkhankulov Rustam wrote:
> Version: 5-19-rc6
>
> In function '__arm_lpae_alloc_pages' pointer 'dev' is compared with
> NULL at [drivers/iommu/io-pgtable-arm.c: 203]. This means that the
> pointer can be NULL.
>
> -----------------------------------------------------------------------
> 203 p = alloc_pages_node(dev ? dev_to_node(dev) : NUMA_NO_NODE,
> 204 gfp | __GFP_ZERO, order);
> -----------------------------------------------------------------------
>
> Then, if cfg->coherent_walk == 0 at [drivers/iommu/io-pgtable-arm.c:
> 209], function 'dma_map_single', which is defined as
> 'dma_map_single_attrs', is called and pointer dev is passed as
> first parameter.
>
> -----------------------------------------------------------------------
> 209 if (!cfg->coherent_walk) {
> 208 dma = dma_map_single(dev, pages, size, DMA_TO_DEVICE);
> -----------------------------------------------------------------------
>
> Therefore, pointer 'dev' passed to function 'dev_driver_string'
> in macro 'dev_WARN_ONCE' at [include/linux/dma-mapping.h: 326],
> where it is dereferenced at [drivers/base/core.c: 2091].
>
> -----------------------------------------------------------------------
> 2083 const char *dev_driver_string(const struct device *dev)
> 2084 {
> 2085 struct device_driver *drv;
> 2086
> ---
> 2091 drv = READ_ONCE(dev->driver);
> -----------------------------------------------------------------------
>
> Thus, if it is possible that 'dev' is null at the same time
> that flag 'coherent_walk' is 0, then NULL pointer will be
> dereferenced.
>
> Should we somehow avoid NULL pointer dereference or is this
> situation impossible and we should remove comparison with NULL?
I think 'dev' is only null in the case of the selftest initcall
(see arm_lpae_do_selftests()), and 'coherent_walk' is always true there.
Will
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [POSSIBLE BUG] iommu/io-pgtable-arm: possible dereferencing of NULL pointer
2022-07-19 17:36 ` Will Deacon
@ 2022-08-01 11:06 ` Robin Murphy
-1 siblings, 0 replies; 8+ messages in thread
From: Robin Murphy @ 2022-08-01 11:06 UTC (permalink / raw)
To: Will Deacon, Subkhankulov Rustam
Cc: Joerg Roedel, linux-arm-kernel, iommu, linux-kernel,
Alexey Khoroshilov, ldv-project
On 2022-07-19 18:36, Will Deacon wrote:
> On Mon, Jul 18, 2022 at 12:20:06PM +0300, Subkhankulov Rustam wrote:
>> Version: 5-19-rc6
>>
>> In function '__arm_lpae_alloc_pages' pointer 'dev' is compared with
>> NULL at [drivers/iommu/io-pgtable-arm.c: 203]. This means that the
>> pointer can be NULL.
>>
>> -----------------------------------------------------------------------
>> 203 p = alloc_pages_node(dev ? dev_to_node(dev) : NUMA_NO_NODE,
>> 204 gfp | __GFP_ZERO, order);
>> -----------------------------------------------------------------------
>>
>> Then, if cfg->coherent_walk == 0 at [drivers/iommu/io-pgtable-arm.c:
>> 209], function 'dma_map_single', which is defined as
>> 'dma_map_single_attrs', is called and pointer dev is passed as
>> first parameter.
>>
>> -----------------------------------------------------------------------
>> 209 if (!cfg->coherent_walk) {
>> 208 dma = dma_map_single(dev, pages, size, DMA_TO_DEVICE);
>> -----------------------------------------------------------------------
>>
>> Therefore, pointer 'dev' passed to function 'dev_driver_string'
>> in macro 'dev_WARN_ONCE' at [include/linux/dma-mapping.h: 326],
>> where it is dereferenced at [drivers/base/core.c: 2091].
>>
>> -----------------------------------------------------------------------
>> 2083 const char *dev_driver_string(const struct device *dev)
>> 2084 {
>> 2085 struct device_driver *drv;
>> 2086
>> ---
>> 2091 drv = READ_ONCE(dev->driver);
>> -----------------------------------------------------------------------
>>
>> Thus, if it is possible that 'dev' is null at the same time
>> that flag 'coherent_walk' is 0, then NULL pointer will be
>> dereferenced.
>>
>> Should we somehow avoid NULL pointer dereference or is this
>> situation impossible and we should remove comparison with NULL?
>
> I think 'dev' is only null in the case of the selftest initcall
> (see arm_lpae_do_selftests()), and 'coherent_walk' is always true there.
Indeed, the intent is that cfg->iommu_dev == NULL is a special case for
the selftest, which must always claim coherency as well for this reason.
I suppose we could add an explicit assertion along those lines in
alloc_pgtable if anyone really thinks it matters.
Cheers,
Robin.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [POSSIBLE BUG] iommu/io-pgtable-arm: possible dereferencing of NULL pointer
@ 2022-08-01 11:06 ` Robin Murphy
0 siblings, 0 replies; 8+ messages in thread
From: Robin Murphy @ 2022-08-01 11:06 UTC (permalink / raw)
To: Will Deacon, Subkhankulov Rustam
Cc: Joerg Roedel, linux-arm-kernel, iommu, linux-kernel,
Alexey Khoroshilov, ldv-project
On 2022-07-19 18:36, Will Deacon wrote:
> On Mon, Jul 18, 2022 at 12:20:06PM +0300, Subkhankulov Rustam wrote:
>> Version: 5-19-rc6
>>
>> In function '__arm_lpae_alloc_pages' pointer 'dev' is compared with
>> NULL at [drivers/iommu/io-pgtable-arm.c: 203]. This means that the
>> pointer can be NULL.
>>
>> -----------------------------------------------------------------------
>> 203 p = alloc_pages_node(dev ? dev_to_node(dev) : NUMA_NO_NODE,
>> 204 gfp | __GFP_ZERO, order);
>> -----------------------------------------------------------------------
>>
>> Then, if cfg->coherent_walk == 0 at [drivers/iommu/io-pgtable-arm.c:
>> 209], function 'dma_map_single', which is defined as
>> 'dma_map_single_attrs', is called and pointer dev is passed as
>> first parameter.
>>
>> -----------------------------------------------------------------------
>> 209 if (!cfg->coherent_walk) {
>> 208 dma = dma_map_single(dev, pages, size, DMA_TO_DEVICE);
>> -----------------------------------------------------------------------
>>
>> Therefore, pointer 'dev' passed to function 'dev_driver_string'
>> in macro 'dev_WARN_ONCE' at [include/linux/dma-mapping.h: 326],
>> where it is dereferenced at [drivers/base/core.c: 2091].
>>
>> -----------------------------------------------------------------------
>> 2083 const char *dev_driver_string(const struct device *dev)
>> 2084 {
>> 2085 struct device_driver *drv;
>> 2086
>> ---
>> 2091 drv = READ_ONCE(dev->driver);
>> -----------------------------------------------------------------------
>>
>> Thus, if it is possible that 'dev' is null at the same time
>> that flag 'coherent_walk' is 0, then NULL pointer will be
>> dereferenced.
>>
>> Should we somehow avoid NULL pointer dereference or is this
>> situation impossible and we should remove comparison with NULL?
>
> I think 'dev' is only null in the case of the selftest initcall
> (see arm_lpae_do_selftests()), and 'coherent_walk' is always true there.
Indeed, the intent is that cfg->iommu_dev == NULL is a special case for
the selftest, which must always claim coherency as well for this reason.
I suppose we could add an explicit assertion along those lines in
alloc_pgtable if anyone really thinks it matters.
Cheers,
Robin.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [POSSIBLE BUG] iommu/io-pgtable-arm: possible dereferencing of NULL pointer
2022-08-01 11:06 ` Robin Murphy
@ 2022-08-01 22:33 ` Alexey Khoroshilov
-1 siblings, 0 replies; 8+ messages in thread
From: Alexey Khoroshilov @ 2022-08-01 22:33 UTC (permalink / raw)
To: Robin Murphy, Will Deacon, Subkhankulov Rustam
Cc: Joerg Roedel, linux-arm-kernel, iommu, linux-kernel, ldv-project
On 01.08.2022 14:06, Robin Murphy wrote:
> On 2022-07-19 18:36, Will Deacon wrote:
>> On Mon, Jul 18, 2022 at 12:20:06PM +0300, Subkhankulov Rustam wrote:
>>> Version: 5-19-rc6
>>>
>>> In function '__arm_lpae_alloc_pages' pointer 'dev' is compared with
>>> NULL at [drivers/iommu/io-pgtable-arm.c: 203]. This means that the
>>> pointer can be NULL.
>>>
>>> -----------------------------------------------------------------------
>>> 203 p = alloc_pages_node(dev ? dev_to_node(dev) : NUMA_NO_NODE,
>>> 204 gfp | __GFP_ZERO, order);
>>> -----------------------------------------------------------------------
>>>
>>> Then, if cfg->coherent_walk == 0 at [drivers/iommu/io-pgtable-arm.c:
>>> 209], function 'dma_map_single', which is defined as
>>> 'dma_map_single_attrs', is called and pointer dev is passed as
>>> first parameter.
>>>
>>> -----------------------------------------------------------------------
>>> 209 if (!cfg->coherent_walk) {
>>> 208 dma = dma_map_single(dev, pages, size, DMA_TO_DEVICE);
>>> -----------------------------------------------------------------------
>>>
>>> Therefore, pointer 'dev' passed to function 'dev_driver_string'
>>> in macro 'dev_WARN_ONCE' at [include/linux/dma-mapping.h: 326],
>>> where it is dereferenced at [drivers/base/core.c: 2091].
>>>
>>> -----------------------------------------------------------------------
>>> 2083 const char *dev_driver_string(const struct device *dev)
>>> 2084 {
>>> 2085 struct device_driver *drv;
>>> 2086
>>> ---
>>> 2091 drv = READ_ONCE(dev->driver);
>>> -----------------------------------------------------------------------
>>>
>>> Thus, if it is possible that 'dev' is null at the same time
>>> that flag 'coherent_walk' is 0, then NULL pointer will be
>>> dereferenced.
>>>
>>> Should we somehow avoid NULL pointer dereference or is this
>>> situation impossible and we should remove comparison with NULL?
>>
>> I think 'dev' is only null in the case of the selftest initcall
>> (see arm_lpae_do_selftests()), and 'coherent_walk' is always true there.
>
> Indeed, the intent is that cfg->iommu_dev == NULL is a special case for
> the selftest, which must always claim coherency as well for this reason.
> I suppose we could add an explicit assertion along those lines in
> alloc_pgtable if anyone really thinks it matters.
Yes, we believe it make sense. It will help to document the intention
and to avoid future questions.
Thank you,
Alexey
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [POSSIBLE BUG] iommu/io-pgtable-arm: possible dereferencing of NULL pointer
@ 2022-08-01 22:33 ` Alexey Khoroshilov
0 siblings, 0 replies; 8+ messages in thread
From: Alexey Khoroshilov @ 2022-08-01 22:33 UTC (permalink / raw)
To: Robin Murphy, Will Deacon, Subkhankulov Rustam
Cc: Joerg Roedel, linux-arm-kernel, iommu, linux-kernel, ldv-project
On 01.08.2022 14:06, Robin Murphy wrote:
> On 2022-07-19 18:36, Will Deacon wrote:
>> On Mon, Jul 18, 2022 at 12:20:06PM +0300, Subkhankulov Rustam wrote:
>>> Version: 5-19-rc6
>>>
>>> In function '__arm_lpae_alloc_pages' pointer 'dev' is compared with
>>> NULL at [drivers/iommu/io-pgtable-arm.c: 203]. This means that the
>>> pointer can be NULL.
>>>
>>> -----------------------------------------------------------------------
>>> 203 p = alloc_pages_node(dev ? dev_to_node(dev) : NUMA_NO_NODE,
>>> 204 gfp | __GFP_ZERO, order);
>>> -----------------------------------------------------------------------
>>>
>>> Then, if cfg->coherent_walk == 0 at [drivers/iommu/io-pgtable-arm.c:
>>> 209], function 'dma_map_single', which is defined as
>>> 'dma_map_single_attrs', is called and pointer dev is passed as
>>> first parameter.
>>>
>>> -----------------------------------------------------------------------
>>> 209 if (!cfg->coherent_walk) {
>>> 208 dma = dma_map_single(dev, pages, size, DMA_TO_DEVICE);
>>> -----------------------------------------------------------------------
>>>
>>> Therefore, pointer 'dev' passed to function 'dev_driver_string'
>>> in macro 'dev_WARN_ONCE' at [include/linux/dma-mapping.h: 326],
>>> where it is dereferenced at [drivers/base/core.c: 2091].
>>>
>>> -----------------------------------------------------------------------
>>> 2083 const char *dev_driver_string(const struct device *dev)
>>> 2084 {
>>> 2085 struct device_driver *drv;
>>> 2086
>>> ---
>>> 2091 drv = READ_ONCE(dev->driver);
>>> -----------------------------------------------------------------------
>>>
>>> Thus, if it is possible that 'dev' is null at the same time
>>> that flag 'coherent_walk' is 0, then NULL pointer will be
>>> dereferenced.
>>>
>>> Should we somehow avoid NULL pointer dereference or is this
>>> situation impossible and we should remove comparison with NULL?
>>
>> I think 'dev' is only null in the case of the selftest initcall
>> (see arm_lpae_do_selftests()), and 'coherent_walk' is always true there.
>
> Indeed, the intent is that cfg->iommu_dev == NULL is a special case for
> the selftest, which must always claim coherency as well for this reason.
> I suppose we could add an explicit assertion along those lines in
> alloc_pgtable if anyone really thinks it matters.
Yes, we believe it make sense. It will help to document the intention
and to avoid future questions.
Thank you,
Alexey
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2022-08-01 22:35 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-07-18 9:20 [POSSIBLE BUG] iommu/io-pgtable-arm: possible dereferencing of NULL pointer Subkhankulov Rustam
2022-07-18 9:20 ` Subkhankulov Rustam
2022-07-19 17:36 ` Will Deacon
2022-07-19 17:36 ` Will Deacon
2022-08-01 11:06 ` Robin Murphy
2022-08-01 11:06 ` Robin Murphy
2022-08-01 22:33 ` Alexey Khoroshilov
2022-08-01 22:33 ` Alexey Khoroshilov
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.