Re: [PATCH v2] udmabuf: Fix a potential (and unlikely) access to unallocated memory
On 2/18/24 11:46, Christophe JAILLET wrote:
If 'list_limit' is set to a very high value, 'lsize' computation could
overflow if 'head.count' is big enough.
In such a case, udmabuf_create() would access to memory beyond 'list'.
Use memdup_array_user() which checks for overflow.
While at it, include .
Fixes: fbb0de795078 ("Add udmabuf misc device")'
I don't think this tag is needed in this case.
Also, please, CC linux-hardening next time.
Signed-off-by: Christophe JAILLET
In any case, LGTM:
Reviewed-by: Gustavo A. R. Silva
Thanks!
--
Gustavo
---
v2: - Use memdup_array_user() [Kees Cook]
- Use sizeof(*list) [Gustavo A. R. Silva]
- Add include
v1:
https://lore.kernel.org/all/3e37f05c7593f1016f0a46de188b3357cbbd0c0b.1695060389.git.christophe.jail...@wanadoo.fr/
Signed-off-by: Christophe JAILLET
---
drivers/dma-buf/udmabuf.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/dma-buf/udmabuf.c b/drivers/dma-buf/udmabuf.c
index c40645999648..5728948ea6f2 100644
--- a/drivers/dma-buf/udmabuf.c
+++ b/drivers/dma-buf/udmabuf.c
@@ -11,6 +11,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -314,14 +315,13 @@ static long udmabuf_ioctl_create_list(struct file *filp,
unsigned long arg)
struct udmabuf_create_list head;
struct udmabuf_create_item *list;
int ret = -EINVAL;
- u32 lsize;
if (copy_from_user(, (void __user *)arg, sizeof(head)))
return -EFAULT;
if (head.count > list_limit)
return -EINVAL;
- lsize = sizeof(struct udmabuf_create_item) * head.count;
- list = memdup_user((void __user *)(arg + sizeof(head)), lsize);
+ list = memdup_array_user((void __user *)(arg + sizeof(head)),
+sizeof(*list), head.count);
if (IS_ERR(list))
return PTR_ERR(list);
Re: [PATCH] drm/xe: Prefer struct_size over open coded arithmetic
On 2/10/24 08:19, Erick Archer wrote:
This is an effort to get rid of all multiplications from allocation
functions in order to prevent integer overflows [1].
As the "q" variable is a pointer to "struct xe_exec_queue" and this
structure ends in a flexible array:
struct xe_exec_queue {
[...]
struct xe_lrc lrc[];
};
the preferred way in the kernel is to use the struct_size() helper to
do the arithmetic instead of the argument "size + size * count" in the
kzalloc() function.
This way, the code is more readable and more safer.
Link:
https://www.kernel.org/doc/html/latest/process/deprecated.html#open-coded-arithmetic-in-allocator-arguments
[1]
Link: https://github.com/KSPP/linux/issues/160 [2]
Signed-off-by: Erick Archer
LGTM:
Reviewed-by: Gustavo A. R. Silva
Thanks!
--
Gustavo
---
drivers/gpu/drm/xe/xe_exec_queue.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/xe/xe_exec_queue.c
b/drivers/gpu/drm/xe/xe_exec_queue.c
index bcfc4127c7c5..f4e53cbccd04 100644
--- a/drivers/gpu/drm/xe/xe_exec_queue.c
+++ b/drivers/gpu/drm/xe/xe_exec_queue.c
@@ -44,7 +44,7 @@ static struct xe_exec_queue *__xe_exec_queue_create(struct
xe_device *xe,
/* only kernel queues can be permanent */
XE_WARN_ON((flags & EXEC_QUEUE_FLAG_PERMANENT) && !(flags &
EXEC_QUEUE_FLAG_KERNEL));
- q = kzalloc(sizeof(*q) + sizeof(struct xe_lrc) * width, GFP_KERNEL);
+ q = kzalloc(struct_size(q, lrc, width), GFP_KERNEL);
if (!q)
return ERR_PTR(-ENOMEM);
--
2.25.1
Re: [PATCH] drm/i915: Add flex arrays to struct i915_syncmap
On 2/8/24 12:13, Erick Archer wrote:
The "struct i915_syncmap" uses a dynamically sized set of trailing
elements. It can use an "u32" array or a "struct i915_syncmap *"
array.
So, use the preferred way in the kernel declaring flexible arrays [1].
Because there are two possibilities for the trailing arrays, it is
necessary to declare a union and use the DECLARE_FLEX_ARRAY macro.
The comment can be removed as the union is now clear enough.
Also, avoid the open-coded arithmetic in the memory allocator functions
[2] using the "struct_size" macro.
Moreover, refactor the "__sync_seqno" and "__sync_child" functions due
to now it is possible to use the union members added to the structure.
This way, it is also possible to avoid the open-coded arithmetic in
pointers.
Link:
https://www.kernel.org/doc/html/next/process/deprecated.html#zero-length-and-one-element-arrays
[1]
Link:
https://www.kernel.org/doc/html/next/process/deprecated.html#open-coded-arithmetic-in-allocator-arguments
[2]
Signed-off-by: Erick Archer
Nice transformation!
LGTM:
Reviewed-by: Gustavo A. R. Silva
Thanks
--
Gustavo
---
drivers/gpu/drm/i915/i915_syncmap.c | 19 ---
1 file changed, 8 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_syncmap.c
b/drivers/gpu/drm/i915/i915_syncmap.c
index 60404dbb2e9f..df6437c37373 100644
--- a/drivers/gpu/drm/i915/i915_syncmap.c
+++ b/drivers/gpu/drm/i915/i915_syncmap.c
@@ -75,13 +75,10 @@ struct i915_syncmap {
unsigned int height;
unsigned int bitmap;
struct i915_syncmap *parent;
- /*
-* Following this header is an array of either seqno or child pointers:
-* union {
-* u32 seqno[KSYNCMAP];
-* struct i915_syncmap *child[KSYNCMAP];
-* };
-*/
+ union {
+ DECLARE_FLEX_ARRAY(u32, seqno);
+ DECLARE_FLEX_ARRAY(struct i915_syncmap *, child);
+ };
};
/**
@@ -99,13 +96,13 @@ void i915_syncmap_init(struct i915_syncmap **root)
static inline u32 *__sync_seqno(struct i915_syncmap *p)
{
GEM_BUG_ON(p->height);
- return (u32 *)(p + 1);
+ return p->seqno;
}
static inline struct i915_syncmap **__sync_child(struct i915_syncmap *p)
{
GEM_BUG_ON(!p->height);
- return (struct i915_syncmap **)(p + 1);
+ return p->child;
}
static inline unsigned int
@@ -200,7 +197,7 @@ __sync_alloc_leaf(struct i915_syncmap *parent, u64 id)
{
struct i915_syncmap *p;
- p = kmalloc(sizeof(*p) + KSYNCMAP * sizeof(u32), GFP_KERNEL);
+ p = kmalloc(struct_size(p, seqno, KSYNCMAP), GFP_KERNEL);
if (unlikely(!p))
return NULL;
@@ -282,7 +279,7 @@ static noinline int __sync_set(struct i915_syncmap **root,
u64 id, u32 seqno)
unsigned int above;
/* Insert a join above the current layer */
- next = kzalloc(sizeof(*next) + KSYNCMAP * sizeof(next),
+ next = kzalloc(struct_size(next, child, KSYNCMAP),
GFP_KERNEL);
if (unlikely(!next))
return -ENOMEM;
--
2.25.1
Re: [GIT PULL] Enable -Wstringop-overflow globally
On 1/27/24 09:11, David Laight wrote: From: Linus Torvalds Sent: 26 January 2024 22:36 On Fri, 26 Jan 2024 at 14:24, Kees Cook wrote: I think xe has some other weird problems too. This may be related (under allocating): ../drivers/gpu/drm/xe/xe_vm.c: In function 'xe_vma_create': ../drivers/gpu/drm/xe/xe_vm.c:806:21: warning: allocation of insufficient size '224' for type 'struct xe_vma' with size '368' [-Walloc-size] 806 | vma = kzalloc(sizeof(*vma) - sizeof(struct xe_userptr), | ^ That code is indeed odd, but there's a comment in the xe_vma definition /** * @userptr: user pointer state, only allocated for VMAs that are * user pointers */ struct xe_userptr userptr; although I agree that it should probably simply be made a final variably-sized array instead (and then you make that array size be 0/1). That entire code is odd. It isn't obvious that the flag values that cause the short allocate are the same ones that control whether the extra data is accessed. Never mind the oddities with the 'flags |= ' assignments int the 'remap next' path. Anyone know how many of these actually get allocated (and their lifetimes)? How much difference would it make to allocate 368 (maybe 384?) bytes instead of 224 (likely 256). [CC+ xen list and maintainers] Probably the xen maintainer can help us out here. -- Gustavo David - Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)
Re: [PATCH] accel/habanalabs: use kcalloc() instead of kzalloc()
On 1/20/24 09:10, Erick Archer wrote: As noted in the "Deprecated Interfaces, Language Features, Attributes, and Conventions" documentation [1], size calculations (especially multiplication) should not be performed in memory allocator (or similar) function arguments due to the risk of them overflowing. This could lead to values wrapping around and a smaller allocation being made than the caller was expecting. Using those allocations could lead to linear overflows of heap memory and other misbehaviors. So, use the purpose specific kcalloc() function instead of the argument size * count in the kzalloc() function. Link: https://www.kernel.org/doc/html/next/process/deprecated.html#open-coded-arithmetic-in-allocator-arguments [1] Link: https://github.com/KSPP/linux/issues/162 Signed-off-by: Erick Archer Reviewed-by: Gustavo A. R. Silva Thanks! -- Gustavo --- drivers/accel/habanalabs/common/mmu/mmu_v1.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/accel/habanalabs/common/mmu/mmu_v1.c b/drivers/accel/habanalabs/common/mmu/mmu_v1.c index d925dc4dd097..e3d42cfead27 100644 --- a/drivers/accel/habanalabs/common/mmu/mmu_v1.c +++ b/drivers/accel/habanalabs/common/mmu/mmu_v1.c @@ -232,7 +232,7 @@ static int dram_default_mapping_init(struct hl_ctx *ctx) /* add hop1 and hop2 */ total_hops = num_of_hop3 + 2; - ctx->dram_default_hops = kzalloc(HL_PTE_SIZE * total_hops, GFP_KERNEL); + ctx->dram_default_hops = kcalloc(total_hops, HL_PTE_SIZE, GFP_KERNEL); if (!ctx->dram_default_hops) return -ENOMEM; -- 2.25.1
Re: [Nouveau] [PATCH][next] nouveau/gsp: replace zero-length array with flex-array member and use __counted_by
On 11/28/23 19:01, Danilo Krummrich wrote: On 11/16/23 20:55, Timur Tabi wrote: On Thu, 2023-11-16 at 20:45 +0100, Danilo Krummrich wrote: As I already mentioned for Timur's patch [2], I'd prefer to get a fix upstream (meaning [1] in this case). Of course, that's probably more up to Timur to tell if this will work out. Don't count on it. I see. Well, I think it's fine. Once we implement a decent abstraction we likely don't need those header files in the kernel anymore. @Gustavo, if you agree I will discard the indentation change when applying the patch to keep the diff as small as possible. No problem. Thanks -- Gustavo
[PATCH][next] nouveau/gsp: replace zero-length array with flex-array member and use __counted_by
Fake flexible arrays (zero-length and one-element arrays) are deprecated,
and should be replaced by flexible-array members. So, replace
zero-length array with a flexible-array member in `struct
PACKED_REGISTRY_TABLE`.
Also annotate array `entries` with `__counted_by()` to prepare for the
coming implementation by GCC and Clang of the `__counted_by` attribute.
Flexible array members annotated with `__counted_by` can have their
accesses bounds-checked at run-time via `CONFIG_UBSAN_BOUNDS` (for array
indexing) and `CONFIG_FORTIFY_SOURCE` (for strcpy/memcpy-family functions).
This fixes multiple -Warray-bounds warnings:
drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c:1069:29: warning: array
subscript 0 is outside array bounds of 'PACKED_REGISTRY_ENTRY[0]'
[-Warray-bounds=]
drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c:1070:29: warning: array
subscript 0 is outside array bounds of 'PACKED_REGISTRY_ENTRY[0]'
[-Warray-bounds=]
drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c:1071:29: warning: array
subscript 0 is outside array bounds of 'PACKED_REGISTRY_ENTRY[0]'
[-Warray-bounds=]
drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c:1072:29: warning: array
subscript 0 is outside array bounds of 'PACKED_REGISTRY_ENTRY[0]'
[-Warray-bounds=]
While there, also make use of the struct_size() helper, and address
checkpatch.pl warning:
WARNING: please, no spaces at the start of a line
This results in no differences in binary output.
Signed-off-by: Gustavo A. R. Silva
---
.../nvrm/535.113.01/nvidia/generated/g_os_nvoc.h | 14 +++---
drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c | 2 +-
2 files changed, 8 insertions(+), 8 deletions(-)
diff --git
a/drivers/gpu/drm/nouveau/include/nvrm/535.113.01/nvidia/generated/g_os_nvoc.h
b/drivers/gpu/drm/nouveau/include/nvrm/535.113.01/nvidia/generated/g_os_nvoc.h
index 754c6af42f30..259b25c2ac6b 100644
---
a/drivers/gpu/drm/nouveau/include/nvrm/535.113.01/nvidia/generated/g_os_nvoc.h
+++
b/drivers/gpu/drm/nouveau/include/nvrm/535.113.01/nvidia/generated/g_os_nvoc.h
@@ -28,17 +28,17 @@
typedef struct PACKED_REGISTRY_ENTRY
{
-NvU32 nameOffset;
-NvU8type;
-NvU32 data;
-NvU32 length;
+ NvU32 nameOffset;
+ NvU8type;
+ NvU32 data;
+ NvU32 length;
} PACKED_REGISTRY_ENTRY;
typedef struct PACKED_REGISTRY_TABLE
{
-NvU32 size;
-NvU32 numEntries;
-PACKED_REGISTRY_ENTRY entries[0];
+ NvU32 size;
+ NvU32 numEntries;
+ PACKED_REGISTRY_ENTRY entries[] __counted_by(numEntries);
} PACKED_REGISTRY_TABLE;
#endif
diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c
b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c
index dc44f5c7833f..228335487af5 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c
@@ -1048,7 +1048,7 @@ r535_gsp_rpc_set_registry(struct nvkm_gsp *gsp)
char *strings;
int str_offset;
int i;
- size_t rpc_size = sizeof(*rpc) + sizeof(rpc->entries[0]) *
NV_GSP_REG_NUM_ENTRIES;
+ size_t rpc_size = struct_size(rpc, entries, NV_GSP_REG_NUM_ENTRIES);
/* add strings + null terminator */
for (i = 0; i < NV_GSP_REG_NUM_ENTRIES; i++)
--
2.34.1
Re: [PATCH][next] dma-buf: Fix NULL pointer dereference in dma_fence_enable_sw_signaling()
On 10/11/23 10:03, Kees Cook wrote:
On Wed, Oct 11, 2023 at 08:03:43AM -0600, Gustavo A. R. Silva wrote:
Currently, a NULL pointer dereference will happen in function
`dma_fence_enable_sw_signaling()` (at line 615), in case `chain`
is not allocated in `mock_chain()` and this function returns
`NULL` (at line 86). See below:
drivers/dma-buf/st-dma-fence-chain.c:
86 chain = mock_chain(NULL, f, 1);
87 if (!chain)
88 err = -ENOMEM;
89
90 dma_fence_enable_sw_signaling(chain);
Instead of the larger patch, should line 88 here just do a "return
-ENOMEM" instead?
Nope. I would have to add a `goto` to skip
`dma_fence_enable_sw_signaling(chain)`.
I originally thought of that, but as other _signaling() functions have
sanity-checks inside, I decided to go with that solution.
This bug has been there since Sep 2022. So, adding a sanity check inside that
function should prevent any other issue of this same kind to enter the codebase
and stay there for years.
--
Gustavo
-Kees
drivers/dma-buf/dma-fence.c:
611 void dma_fence_enable_sw_signaling(struct dma_fence *fence)
612 {
613 unsigned long flags;
614
615 spin_lock_irqsave(fence->lock, flags);
^^^
|
NULL pointer reference
if fence == NULL
616 __dma_fence_enable_signaling(fence);
617 spin_unlock_irqrestore(fence->lock, flags);
618 }
Fix this by adding a NULL check before dereferencing `fence` in
`dma_fence_enable_sw_signaling()`. This will prevent any other NULL
pointer dereference when the `fence` passed as an argument is `NULL`.
Addresses-Coverity: ("Dereference after null check")
Fixes: d62c43a953ce ("dma-buf: Enable signaling on fence for selftests")
Cc: sta...@vger.kernel.org
Signed-off-by: Gustavo A. R. Silva
---
drivers/dma-buf/dma-fence.c | 9 -
include/linux/dma-fence.h | 2 +-
2 files changed, 9 insertions(+), 2 deletions(-)
diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c
index 8aa8f8cb7071..4d2f13560d0f 100644
--- a/drivers/dma-buf/dma-fence.c
+++ b/drivers/dma-buf/dma-fence.c
@@ -607,14 +607,21 @@ static bool __dma_fence_enable_signaling(struct dma_fence
*fence)
* This will request for sw signaling to be enabled, to make the fence
* complete as soon as possible. This calls _fence_ops.enable_signaling
* internally.
+ *
+ * Returns 0 on success and a negative error value when @fence is NULL.
*/
-void dma_fence_enable_sw_signaling(struct dma_fence *fence)
+int dma_fence_enable_sw_signaling(struct dma_fence *fence)
{
unsigned long flags;
+ if (!fence)
+ return -EINVAL;
+
spin_lock_irqsave(fence->lock, flags);
__dma_fence_enable_signaling(fence);
spin_unlock_irqrestore(fence->lock, flags);
+
+ return 0;
}
EXPORT_SYMBOL(dma_fence_enable_sw_signaling);
diff --git a/include/linux/dma-fence.h b/include/linux/dma-fence.h
index ebe78bd3d121..1e4025e925e6 100644
--- a/include/linux/dma-fence.h
+++ b/include/linux/dma-fence.h
@@ -399,7 +399,7 @@ int dma_fence_add_callback(struct dma_fence *fence,
dma_fence_func_t func);
bool dma_fence_remove_callback(struct dma_fence *fence,
struct dma_fence_cb *cb);
-void dma_fence_enable_sw_signaling(struct dma_fence *fence);
+int dma_fence_enable_sw_signaling(struct dma_fence *fence);
/**
* dma_fence_is_signaled_locked - Return an indication if the fence
--
2.34.1
[PATCH][next] dma-buf: Fix NULL pointer dereference in dma_fence_enable_sw_signaling()
Currently, a NULL pointer dereference will happen in function
`dma_fence_enable_sw_signaling()` (at line 615), in case `chain`
is not allocated in `mock_chain()` and this function returns
`NULL` (at line 86). See below:
drivers/dma-buf/st-dma-fence-chain.c:
86 chain = mock_chain(NULL, f, 1);
87 if (!chain)
88 err = -ENOMEM;
89
90 dma_fence_enable_sw_signaling(chain);
drivers/dma-buf/dma-fence.c:
611 void dma_fence_enable_sw_signaling(struct dma_fence *fence)
612 {
613 unsigned long flags;
614
615 spin_lock_irqsave(fence->lock, flags);
^^^
|
NULL pointer reference
if fence == NULL
616 __dma_fence_enable_signaling(fence);
617 spin_unlock_irqrestore(fence->lock, flags);
618 }
Fix this by adding a NULL check before dereferencing `fence` in
`dma_fence_enable_sw_signaling()`. This will prevent any other NULL
pointer dereference when the `fence` passed as an argument is `NULL`.
Addresses-Coverity: ("Dereference after null check")
Fixes: d62c43a953ce ("dma-buf: Enable signaling on fence for selftests")
Cc: sta...@vger.kernel.org
Signed-off-by: Gustavo A. R. Silva
---
drivers/dma-buf/dma-fence.c | 9 -
include/linux/dma-fence.h | 2 +-
2 files changed, 9 insertions(+), 2 deletions(-)
diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c
index 8aa8f8cb7071..4d2f13560d0f 100644
--- a/drivers/dma-buf/dma-fence.c
+++ b/drivers/dma-buf/dma-fence.c
@@ -607,14 +607,21 @@ static bool __dma_fence_enable_signaling(struct dma_fence
*fence)
* This will request for sw signaling to be enabled, to make the fence
* complete as soon as possible. This calls _fence_ops.enable_signaling
* internally.
+ *
+ * Returns 0 on success and a negative error value when @fence is NULL.
*/
-void dma_fence_enable_sw_signaling(struct dma_fence *fence)
+int dma_fence_enable_sw_signaling(struct dma_fence *fence)
{
unsigned long flags;
+ if (!fence)
+ return -EINVAL;
+
spin_lock_irqsave(fence->lock, flags);
__dma_fence_enable_signaling(fence);
spin_unlock_irqrestore(fence->lock, flags);
+
+ return 0;
}
EXPORT_SYMBOL(dma_fence_enable_sw_signaling);
diff --git a/include/linux/dma-fence.h b/include/linux/dma-fence.h
index ebe78bd3d121..1e4025e925e6 100644
--- a/include/linux/dma-fence.h
+++ b/include/linux/dma-fence.h
@@ -399,7 +399,7 @@ int dma_fence_add_callback(struct dma_fence *fence,
dma_fence_func_t func);
bool dma_fence_remove_callback(struct dma_fence *fence,
struct dma_fence_cb *cb);
-void dma_fence_enable_sw_signaling(struct dma_fence *fence);
+int dma_fence_enable_sw_signaling(struct dma_fence *fence);
/**
* dma_fence_is_signaled_locked - Return an indication if the fence
--
2.34.1
Re: [PATCH] drm/i915/guc: Annotate struct ct_incoming_msg with __counted_by
On 10/6/23 22:17, Kees Cook wrote:
Prepare for the coming implementation by GCC and Clang of the __counted_by
attribute. Flexible array members annotated with __counted_by can have
their accesses bounds-checked at run-time via CONFIG_UBSAN_BOUNDS (for
array indexing) and CONFIG_FORTIFY_SOURCE (for strcpy/memcpy-family
functions).
As found with Coccinelle[1], add __counted_by for struct ct_incoming_msg.
Cc: Jani Nikula
Cc: Joonas Lahtinen
Cc: Rodrigo Vivi
Cc: Tvrtko Ursulin
Cc: David Airlie
Cc: Daniel Vetter
Cc: "Gustavo A. R. Silva"
Cc: John Harrison
Cc: Matthew Brost
Cc: Michal Wajdeczko
Cc: Matt Roper
Cc: intel-...@lists.freedesktop.org
Cc: dri-devel@lists.freedesktop.org
Cc: linux-harden...@vger.kernel.org
Link:
https://github.com/kees/kernel-tools/blob/trunk/coccinelle/examples/counted_by.cocci
[1]
Signed-off-by: Kees Cook
Reviewed-by: Gustavo A. R. Silva
Thanks!
--
Gustavo
---
drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c
b/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c
index 6e22af31513a..c33210ead1ef 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c
@@ -96,7 +96,7 @@ struct ct_request {
struct ct_incoming_msg {
struct list_head link;
u32 size;
- u32 msg[];
+ u32 msg[] __counted_by(size);
};
enum { CTB_SEND = 0, CTB_RECV = 1 };
Re: [PATCH 5/9] drm/nouveau/pm: Annotate struct nvkm_perfdom with __counted_by
On 9/22/23 11:32, Kees Cook wrote:
Prepare for the coming implementation by GCC and Clang of the __counted_by
attribute. Flexible array members annotated with __counted_by can have
their accesses bounds-checked at run-time checking via CONFIG_UBSAN_BOUNDS
(for array indexing) and CONFIG_FORTIFY_SOURCE (for strcpy/memcpy-family
functions).
As found with Coccinelle[1], add __counted_by for struct nvkm_perfdom.
[1]
https://github.com/kees/kernel-tools/blob/trunk/coccinelle/examples/counted_by.cocci
Cc: Ben Skeggs
Cc: Karol Herbst
Cc: Lyude Paul
Cc: David Airlie
Cc: Daniel Vetter
Cc: dri-devel@lists.freedesktop.org
Cc: nouv...@lists.freedesktop.org
Signed-off-by: Kees Cook
Reviewed-by: Gustavo A. R. Silva
Thanks
--
Gustavo
---
drivers/gpu/drm/nouveau/nvkm/engine/pm/priv.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/nouveau/nvkm/engine/pm/priv.h
b/drivers/gpu/drm/nouveau/nvkm/engine/pm/priv.h
index 6ae25d3e7f45..c011227f7052 100644
--- a/drivers/gpu/drm/nouveau/nvkm/engine/pm/priv.h
+++ b/drivers/gpu/drm/nouveau/nvkm/engine/pm/priv.h
@@ -82,7 +82,7 @@ struct nvkm_perfdom {
u8 mode;
u32 clk;
u16 signal_nr;
- struct nvkm_perfsig signal[];
+ struct nvkm_perfsig signal[] __counted_by(signal_nr);
};
struct nvkm_funcdom {
Re: [PATCH 7/9] drm/virtio: Annotate struct virtio_gpu_object_array with __counted_by
On 9/22/23 11:32, Kees Cook wrote:
Prepare for the coming implementation by GCC and Clang of the __counted_by
attribute. Flexible array members annotated with __counted_by can have
their accesses bounds-checked at run-time checking via CONFIG_UBSAN_BOUNDS
(for array indexing) and CONFIG_FORTIFY_SOURCE (for strcpy/memcpy-family
functions).
As found with Coccinelle[1], add __counted_by for struct
virtio_gpu_object_array.
[1]
https://github.com/kees/kernel-tools/blob/trunk/coccinelle/examples/counted_by.cocci
Cc: David Airlie
Cc: Gerd Hoffmann
Cc: Gurchetan Singh
Cc: Chia-I Wu
Cc: Daniel Vetter
Cc: dri-devel@lists.freedesktop.org
Cc: virtualizat...@lists.linux-foundation.org
Signed-off-by: Kees Cook
Reviewed-by: Gustavo A. R. Silva
Thanks
--
Gustavo
---
drivers/gpu/drm/virtio/virtgpu_drv.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h
b/drivers/gpu/drm/virtio/virtgpu_drv.h
index 8513b671f871..96365a772f77 100644
--- a/drivers/gpu/drm/virtio/virtgpu_drv.h
+++ b/drivers/gpu/drm/virtio/virtgpu_drv.h
@@ -119,7 +119,7 @@ struct virtio_gpu_object_array {
struct ww_acquire_ctx ticket;
struct list_head next;
u32 nents, total;
- struct drm_gem_object *objs[];
+ struct drm_gem_object *objs[] __counted_by(total);
};
struct virtio_gpu_vbuffer;
Re: [PATCH] video: mmp: Annotate struct mmp_path with __counted_by
On 9/22/23 11:51, Kees Cook wrote:
Prepare for the coming implementation by GCC and Clang of the __counted_by
attribute. Flexible array members annotated with __counted_by can have
their accesses bounds-checked at run-time checking via CONFIG_UBSAN_BOUNDS
(for array indexing) and CONFIG_FORTIFY_SOURCE (for strcpy/memcpy-family
functions).
As found with Coccinelle[1], add __counted_by for struct mmp_path.
[1]
https://github.com/kees/kernel-tools/blob/trunk/coccinelle/examples/counted_by.cocci
Cc: Helge Deller
Cc: linux-fb...@vger.kernel.org
Cc: dri-devel@lists.freedesktop.org
Signed-off-by: Kees Cook
Reviewed-by: Gustavo A. R. Silva
Thanks
--
Gustavo
---
include/video/mmp_disp.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/video/mmp_disp.h b/include/video/mmp_disp.h
index 77252cb46361..a722dcbf5073 100644
--- a/include/video/mmp_disp.h
+++ b/include/video/mmp_disp.h
@@ -231,7 +231,7 @@ struct mmp_path {
/* layers */
int overlay_num;
- struct mmp_overlay overlays[];
+ struct mmp_overlay overlays[] __counted_by(overlay_num);
};
extern struct mmp_path *mmp_get_path(const char *name);
Re: [PATCH] video: fbdev: mmp: Annotate struct mmphw_ctrl with __counted_by
On 9/22/23 11:51, Kees Cook wrote:
Prepare for the coming implementation by GCC and Clang of the __counted_by
attribute. Flexible array members annotated with __counted_by can have
their accesses bounds-checked at run-time checking via CONFIG_UBSAN_BOUNDS
(for array indexing) and CONFIG_FORTIFY_SOURCE (for strcpy/memcpy-family
functions).
As found with Coccinelle[1], add __counted_by for struct mmphw_ctrl.
[1]
https://github.com/kees/kernel-tools/blob/trunk/coccinelle/examples/counted_by.cocci
Cc: Helge Deller
Cc: linux-fb...@vger.kernel.org
Cc: dri-devel@lists.freedesktop.org
Signed-off-by: Kees Cook
Reviewed-by: Gustavo A. R. Silva
Thanks
--
Gustavo
---
drivers/video/fbdev/mmp/hw/mmp_ctrl.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/video/fbdev/mmp/hw/mmp_ctrl.h
b/drivers/video/fbdev/mmp/hw/mmp_ctrl.h
index 167585a889d3..719b99a9bc77 100644
--- a/drivers/video/fbdev/mmp/hw/mmp_ctrl.h
+++ b/drivers/video/fbdev/mmp/hw/mmp_ctrl.h
@@ -1406,7 +1406,7 @@ struct mmphw_ctrl {
/*pathes*/
int path_num;
- struct mmphw_path_plat path_plats[];
+ struct mmphw_path_plat path_plats[] __counted_by(path_num);
};
static inline int overlay_is_vid(struct mmp_overlay *overlay)
Re: [PATCH 1/9] drm/amd/pm: Annotate struct smu10_voltage_dependency_table with __counted_by
On 9/22/23 11:32, Kees Cook wrote:
Prepare for the coming implementation by GCC and Clang of the __counted_by
attribute. Flexible array members annotated with __counted_by can have
their accesses bounds-checked at run-time checking via CONFIG_UBSAN_BOUNDS
(for array indexing) and CONFIG_FORTIFY_SOURCE (for strcpy/memcpy-family
functions).
As found with Coccinelle[1], add __counted_by for struct
smu10_voltage_dependency_table.
[1]
https://github.com/kees/kernel-tools/blob/trunk/coccinelle/examples/counted_by.cocci
Cc: Evan Quan
Cc: Alex Deucher
Cc: "Christian König"
Cc: "Pan, Xinhui"
Cc: David Airlie
Cc: Daniel Vetter
Cc: Xiaojian Du
Cc: Huang Rui
Cc: Kevin Wang
Cc: amd-...@lists.freedesktop.org
Cc: dri-devel@lists.freedesktop.org
Signed-off-by: Kees Cook
Reviewed-by: Gustavo A. R. Silva
Thanks
--
Gustavo
---
drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu10_hwmgr.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu10_hwmgr.h
b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu10_hwmgr.h
index 808e0ecbe1f0..42adc2a3dcbc 100644
--- a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu10_hwmgr.h
+++ b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu10_hwmgr.h
@@ -192,7 +192,7 @@ struct smu10_clock_voltage_dependency_record {
struct smu10_voltage_dependency_table {
uint32_t count;
- struct smu10_clock_voltage_dependency_record entries[];
+ struct smu10_clock_voltage_dependency_record entries[]
__counted_by(count);
};
struct smu10_clock_voltage_information {
Re: [PATCH 6/9] drm/vc4: Annotate struct vc4_perfmon with __counted_by
On 9/22/23 11:32, Kees Cook wrote:
Prepare for the coming implementation by GCC and Clang of the __counted_by
attribute. Flexible array members annotated with __counted_by can have
their accesses bounds-checked at run-time checking via CONFIG_UBSAN_BOUNDS
(for array indexing) and CONFIG_FORTIFY_SOURCE (for strcpy/memcpy-family
functions).
As found with Coccinelle[1], add __counted_by for struct vc4_perfmon.
[1]
https://github.com/kees/kernel-tools/blob/trunk/coccinelle/examples/counted_by.cocci
Cc: Emma Anholt
Cc: Maxime Ripard
Cc: David Airlie
Cc: Daniel Vetter
Cc: dri-devel@lists.freedesktop.org
Signed-off-by: Kees Cook
Reviewed-by: Gustavo A. R. Silva
Thanks
--
Gustavo
---
drivers/gpu/drm/vc4/vc4_drv.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/vc4/vc4_drv.h b/drivers/gpu/drm/vc4/vc4_drv.h
index bf66499765fb..ab61e96e7e14 100644
--- a/drivers/gpu/drm/vc4/vc4_drv.h
+++ b/drivers/gpu/drm/vc4/vc4_drv.h
@@ -76,7 +76,7 @@ struct vc4_perfmon {
* Note that counter values can't be reset, but you can fake a reset by
* destroying the perfmon and creating a new one.
*/
- u64 counters[];
+ u64 counters[] __counted_by(ncounters);
};
struct vc4_dev {
Re: [PATCH 4/9] drm/msm/dpu: Annotate struct dpu_hw_intr with __counted_by
On 9/22/23 11:32, Kees Cook wrote:
Prepare for the coming implementation by GCC and Clang of the __counted_by
attribute. Flexible array members annotated with __counted_by can have
their accesses bounds-checked at run-time checking via CONFIG_UBSAN_BOUNDS
(for array indexing) and CONFIG_FORTIFY_SOURCE (for strcpy/memcpy-family
functions).
As found with Coccinelle[1], add __counted_by for struct dpu_hw_intr.
[1]
https://github.com/kees/kernel-tools/blob/trunk/coccinelle/examples/counted_by.cocci
Cc: Rob Clark
Cc: Abhinav Kumar
Cc: Dmitry Baryshkov
Cc: Sean Paul
Cc: Marijn Suijten
Cc: David Airlie
Cc: Daniel Vetter
Cc: Bjorn Andersson
Cc: linux-arm-...@vger.kernel.org
Cc: dri-devel@lists.freedesktop.org
Cc: freedr...@lists.freedesktop.org
Signed-off-by: Kees Cook
Reviewed-by: Gustavo A. R. Silva
Thanks
--
Gustavo
---
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.h
b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.h
index dab761e54863..50cf9523d367 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.h
@@ -61,7 +61,7 @@ struct dpu_hw_intr {
void (*cb)(void *arg, int irq_idx);
void *arg;
atomic_t count;
- } irq_tbl[];
+ } irq_tbl[] __counted_by(total_irqs);
};
/**
Re: [PATCH 3/9] drm/i915/selftests: Annotate struct perf_series with __counted_by
On 9/22/23 11:32, Kees Cook wrote:
Prepare for the coming implementation by GCC and Clang of the __counted_by
attribute. Flexible array members annotated with __counted_by can have
their accesses bounds-checked at run-time checking via CONFIG_UBSAN_BOUNDS
(for array indexing) and CONFIG_FORTIFY_SOURCE (for strcpy/memcpy-family
functions).
As found with Coccinelle[1], add __counted_by for struct perf_series.
[1]
https://github.com/kees/kernel-tools/blob/trunk/coccinelle/examples/counted_by.cocci
Cc: Jani Nikula
Cc: Joonas Lahtinen
Cc: Rodrigo Vivi
Cc: Tvrtko Ursulin
Cc: David Airlie
Cc: Daniel Vetter
Cc: Chris Wilson
Cc: John Harrison
Cc: Andi Shyti
Cc: Matthew Brost
Cc: intel-...@lists.freedesktop.org
Cc: dri-devel@lists.freedesktop.org
Signed-off-by: Kees Cook
Reviewed-by: Gustavo A. R. Silva
Thanks
--
Gustavo
---
drivers/gpu/drm/i915/selftests/i915_request.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/selftests/i915_request.c
b/drivers/gpu/drm/i915/selftests/i915_request.c
index a9b79888c193..acae30a04a94 100644
--- a/drivers/gpu/drm/i915/selftests/i915_request.c
+++ b/drivers/gpu/drm/i915/selftests/i915_request.c
@@ -1924,7 +1924,7 @@ struct perf_stats {
struct perf_series {
struct drm_i915_private *i915;
unsigned int nengines;
- struct intel_context *ce[];
+ struct intel_context *ce[] __counted_by(nengines);
};
static int cmp_u32(const void *A, const void *B)
Re: [PATCH 2/9] drm/amdgpu/discovery: Annotate struct ip_hw_instance with __counted_by
On 9/22/23 11:32, Kees Cook wrote:
Prepare for the coming implementation by GCC and Clang of the __counted_by
attribute. Flexible array members annotated with __counted_by can have
their accesses bounds-checked at run-time checking via CONFIG_UBSAN_BOUNDS
(for array indexing) and CONFIG_FORTIFY_SOURCE (for strcpy/memcpy-family
functions).
As found with Coccinelle[1], add __counted_by for struct ip_hw_instance.
[1]
https://github.com/kees/kernel-tools/blob/trunk/coccinelle/examples/counted_by.cocci
Cc: Alex Deucher
Cc: "Christian König"
Cc: "Pan, Xinhui"
Cc: David Airlie
Cc: Daniel Vetter
Cc: Hawking Zhang
Cc: amd-...@lists.freedesktop.org
Cc: dri-devel@lists.freedesktop.org
Signed-off-by: Kees Cook
Reviewed-by: Gustavo A. R. Silva
Thanks
--
Gustavo
---
drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
index d1bc7b212520..be4c97a3d7bf 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
@@ -662,7 +662,7 @@ struct ip_hw_instance {
u8 harvest;
int num_base_addresses;
- u32 base_addr[];
+ u32 base_addr[] __counted_by(num_base_addresses);
};
struct ip_hw_id {
Re: [PATCH] udmabuf: Fix a potential (and unlikely) access to unallocated memory
On 9/18/23 12:46, Christophe JAILLET wrote:
If 'list_limit' is set to a very high value, 'lsize' computation could
overflow if 'head.count' is big enough.
In such a case, udmabuf_create() will access to memory beyond 'list'.
Use size_mul() to saturate the value, and have memdup_user() fail.
Fixes: fbb0de795078 ("Add udmabuf misc device")
Signed-off-by: Christophe JAILLET
---
drivers/dma-buf/udmabuf.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/dma-buf/udmabuf.c b/drivers/dma-buf/udmabuf.c
index c40645999648..fb4c4b5b3332 100644
--- a/drivers/dma-buf/udmabuf.c
+++ b/drivers/dma-buf/udmabuf.c
@@ -314,13 +314,13 @@ static long udmabuf_ioctl_create_list(struct file *filp,
unsigned long arg)
struct udmabuf_create_list head;
struct udmabuf_create_item *list;
int ret = -EINVAL;
- u32 lsize;
+ size_t lsize;
if (copy_from_user(, (void __user *)arg, sizeof(head)))
return -EFAULT;
if (head.count > list_limit)
return -EINVAL;
- lsize = sizeof(struct udmabuf_create_item) * head.count;
+ lsize = size_mul(sizeof(struct udmabuf_create_item), head.count);
list = memdup_user((void __user *)(arg + sizeof(head)), lsize);
if (IS_ERR(list))
return PTR_ERR(list);
How about this, and we get rid of `lsize`:
diff --git a/drivers/dma-buf/udmabuf.c b/drivers/dma-buf/udmabuf.c
index c40645999648..5cf9d849aaa8 100644
--- a/drivers/dma-buf/udmabuf.c
+++ b/drivers/dma-buf/udmabuf.c
@@ -314,14 +314,13 @@ static long udmabuf_ioctl_create_list(struct file *filp,
unsigned long arg)
struct udmabuf_create_list head;
struct udmabuf_create_item *list;
int ret = -EINVAL;
- u32 lsize;
if (copy_from_user(, (void __user *)arg, sizeof(head)))
return -EFAULT;
if (head.count > list_limit)
return -EINVAL;
- lsize = sizeof(struct udmabuf_create_item) * head.count;
- list = memdup_user((void __user *)(arg + sizeof(head)), lsize);
+ list = memdup_user((void __user *)(arg + sizeof(head)),
+ size_mul(sizeof(*list), head.count));
if (IS_ERR(list))
return PTR_ERR(list);
--
Gustavo
Re: [PATCH][next] drm/gud: Use size_add() in call to struct_size()
On 9/15/23 12:52, Kees Cook wrote:
On Fri, Sep 15, 2023 at 12:43:20PM -0600, Gustavo A. R. Silva wrote:
If, for any reason, the open-coded arithmetic causes a wraparound, the
protection that `struct_size()` adds against potential integer overflows
is defeated. Fix this by hardening call to `struct_size()` with `size_add()`.
Fixes: 40e1a70b4aed ("drm: Add GUD USB Display driver")
Signed-off-by: Gustavo A. R. Silva
---
drivers/gpu/drm/gud/gud_pipe.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/gud/gud_pipe.c b/drivers/gpu/drm/gud/gud_pipe.c
index d2f199ea3c11..a02f75be81f0 100644
--- a/drivers/gpu/drm/gud/gud_pipe.c
+++ b/drivers/gpu/drm/gud/gud_pipe.c
@@ -503,7 +503,7 @@ int gud_pipe_check(struct drm_simple_display_pipe *pipe,
return -ENOENT;
len = struct_size(req, properties,
- GUD_PROPERTIES_MAX_NUM +
GUD_CONNECTOR_PROPERTIES_MAX_NUM);
+ size_add(GUD_PROPERTIES_MAX_NUM,
GUD_CONNECTOR_PROPERTIES_MAX_NUM));
There are both constant expressions, so there's not too much value in
wrapping them with size_add(), but for maintaining a common coding style
for dealing with allocation sizes, I can be convinced of the change. :)
Yep; I've found a mix of constant expressions and variables doing open-coded
arithmetic
in `struct_size()`, so I'm sending them all.
Reviewed-by: Kees Cook
Thanks!
--
Gustavo
req = kzalloc(len, GFP_KERNEL);
if (!req)
return -ENOMEM;
--
2.34.1
[PATCH][next] drm/gud: Use size_add() in call to struct_size()
If, for any reason, the open-coded arithmetic causes a wraparound, the
protection that `struct_size()` adds against potential integer overflows
is defeated. Fix this by hardening call to `struct_size()` with `size_add()`.
Fixes: 40e1a70b4aed ("drm: Add GUD USB Display driver")
Signed-off-by: Gustavo A. R. Silva
---
drivers/gpu/drm/gud/gud_pipe.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/gud/gud_pipe.c b/drivers/gpu/drm/gud/gud_pipe.c
index d2f199ea3c11..a02f75be81f0 100644
--- a/drivers/gpu/drm/gud/gud_pipe.c
+++ b/drivers/gpu/drm/gud/gud_pipe.c
@@ -503,7 +503,7 @@ int gud_pipe_check(struct drm_simple_display_pipe *pipe,
return -ENOENT;
len = struct_size(req, properties,
- GUD_PROPERTIES_MAX_NUM +
GUD_CONNECTOR_PROPERTIES_MAX_NUM);
+ size_add(GUD_PROPERTIES_MAX_NUM,
GUD_CONNECTOR_PROPERTIES_MAX_NUM));
req = kzalloc(len, GFP_KERNEL);
if (!req)
return -ENOMEM;
--
2.34.1
Re: [PATCH] clk: Annotate struct clk_hw_onecell_data with __counted_by
On 8/17/23 14:30, Kees Cook wrote:
Prepare for the coming implementation by GCC and Clang of the __counted_by
attribute. Flexible array members annotated with __counted_by can have
their accesses bounds-checked at run-time checking via CONFIG_UBSAN_BOUNDS
(for array indexing) and CONFIG_FORTIFY_SOURCE (for strcpy/memcpy-family
functions).
As found with Coccinelle[1], add __counted_by for struct clk_hw_onecell_data.
Additionally, since the element count member must be set before accessing
the annotated flexible array member, move its initialization earlier.
[1]
https://github.com/kees/kernel-tools/blob/trunk/coccinelle/examples/counted_by.cocci
Cc: Michael Turquette
Cc: Stephen Boyd
Cc: Joel Stanley
Cc: Andrew Jeffery
Cc: Taichi Sugaya
Cc: Takao Orito
Cc: Qin Jian
Cc: Andrew Lunn
Cc: Gregory Clement
Cc: Sebastian Hesselbarth
Cc: Andy Gross
Cc: Bjorn Andersson
Cc: Konrad Dybcio
Cc: Sergio Paracuellos
Cc: Matthias Brugger
Cc: AngeloGioacchino Del Regno
Cc: Maxime Ripard
Cc: Chen-Yu Tsai
Cc: Jernej Skrabec
Cc: David Airlie
Cc: Daniel Vetter
Cc: Samuel Holland
Cc: Vinod Koul
Cc: Kishon Vijay Abraham I
Cc: linux-...@vger.kernel.org
Cc: linux-arm-ker...@lists.infradead.org
Cc: linux-asp...@lists.ozlabs.org
Cc: linux-arm-...@vger.kernel.org
Cc: linux-media...@lists.infradead.org
Cc: dri-devel@lists.freedesktop.org
Cc: linux-su...@lists.linux.dev
Cc: linux-...@lists.infradead.org
Signed-off-by: Kees Cook
Reviewed-by: Gustavo A. R. Silva
Thanks
--
Gustavo
---
drivers/clk/clk-aspeed.c| 3 +--
drivers/clk/clk-ast2600.c | 2 +-
drivers/clk/clk-gemini.c| 2 +-
drivers/clk/clk-milbeaut.c | 3 +--
drivers/clk/clk-sp7021.c| 3 +--
drivers/clk/mvebu/cp110-system-controller.c | 2 +-
drivers/clk/qcom/clk-cpu-8996.c | 2 +-
drivers/clk/ralink/clk-mt7621.c | 3 +--
drivers/gpu/drm/sun4i/sun8i_tcon_top.c | 3 +--
drivers/phy/qualcomm/phy-qcom-edp.c | 2 +-
include/linux/clk-provider.h| 2 +-
11 files changed, 11 insertions(+), 16 deletions(-)
diff --git a/drivers/clk/clk-aspeed.c b/drivers/clk/clk-aspeed.c
index 284710adaef5..ff84191d0fe8 100644
--- a/drivers/clk/clk-aspeed.c
+++ b/drivers/clk/clk-aspeed.c
@@ -701,6 +701,7 @@ static void __init aspeed_cc_init(struct device_node *np)
GFP_KERNEL);
if (!aspeed_clk_data)
return;
+ aspeed_clk_data->num = ASPEED_NUM_CLKS;
/*
* This way all clocks fetched before the platform device probes,
@@ -732,8 +733,6 @@ static void __init aspeed_cc_init(struct device_node *np)
aspeed_ast2500_cc(map);
else
pr_err("unknown platform, failed to add clocks\n");
-
- aspeed_clk_data->num = ASPEED_NUM_CLKS;
ret = of_clk_add_hw_provider(np, of_clk_hw_onecell_get,
aspeed_clk_data);
if (ret)
pr_err("failed to add DT provider: %d\n", ret);
diff --git a/drivers/clk/clk-ast2600.c b/drivers/clk/clk-ast2600.c
index f9e27f95a967..909c3137c428 100644
--- a/drivers/clk/clk-ast2600.c
+++ b/drivers/clk/clk-ast2600.c
@@ -839,6 +839,7 @@ static void __init aspeed_g6_cc_init(struct device_node *np)
ASPEED_G6_NUM_CLKS), GFP_KERNEL);
if (!aspeed_g6_clk_data)
return;
+ aspeed_g6_clk_data->num = ASPEED_G6_NUM_CLKS;
/*
* This way all clocks fetched before the platform device probes,
@@ -860,7 +861,6 @@ static void __init aspeed_g6_cc_init(struct device_node *np)
}
aspeed_g6_cc(map);
- aspeed_g6_clk_data->num = ASPEED_G6_NUM_CLKS;
ret = of_clk_add_hw_provider(np, of_clk_hw_onecell_get,
aspeed_g6_clk_data);
if (ret)
pr_err("failed to add DT provider: %d\n", ret);
diff --git a/drivers/clk/clk-gemini.c b/drivers/clk/clk-gemini.c
index a23fa6d47ef1..2572d15aadd0 100644
--- a/drivers/clk/clk-gemini.c
+++ b/drivers/clk/clk-gemini.c
@@ -404,6 +404,7 @@ static void __init gemini_cc_init(struct device_node *np)
GFP_KERNEL);
if (!gemini_clk_data)
return;
+ gemini_clk_data->num = GEMINI_NUM_CLKS;
/*
* This way all clock fetched before the platform device probes,
@@ -457,7 +458,6 @@ static void __init gemini_cc_init(struct device_node *np)
gemini_clk_data->hws[GEMINI_CLK_APB] = hw;
/* Register the clocks to be accessed by the device tree */
- gemini_clk_data->num = GEMINI_NUM_CLKS;
of_clk_add_hw_provider(np, of_clk_hw_onecell_get, gemini_clk_data);
}
CLK_OF_DECLARE_DRIVER(gemini_cc, "cortina,gemini-syscon", gemini_cc_init);
diff --git a/drivers/clk/clk-milbeaut.c b/drivers/clk/clk-milbeaut.c
index 050fd4fb588f..18c20aff45f7 100644
--- a/drivers/clk/clk-milbeaut.c
+++ b/driv
[PATCH 2/2][next] nouveau/svm: Split assignment from if conditional
Fix checkpatch.pl ERROR: do not use assignment in if condition. Signed-off-by: Gustavo A. R. Silva --- drivers/gpu/drm/nouveau/nouveau_svm.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/nouveau/nouveau_svm.c b/drivers/gpu/drm/nouveau/nouveau_svm.c index 00444ad82d18..cc03e0c22ff3 100644 --- a/drivers/gpu/drm/nouveau/nouveau_svm.c +++ b/drivers/gpu/drm/nouveau/nouveau_svm.c @@ -1063,7 +1063,8 @@ nouveau_svm_init(struct nouveau_drm *drm) if (drm->client.device.info.family > NV_DEVICE_INFO_V0_PASCAL) return; - if (!(drm->svm = svm = kzalloc(struct_size(drm->svm, buffer, 1), GFP_KERNEL))) + drm->svm = svm = kzalloc(struct_size(drm->svm, buffer, 1), GFP_KERNEL); + if (!drm->svm) return; drm->svm->drm = drm; -- 2.34.1
[PATCH 1/2][next] nouveau/svm: Replace one-element array with flexible-array member in struct nouveau_svm
One-element and zero-length arrays are deprecated. So, replace
one-element array in struct nouveau_svm with flexible-array member.
This results in no differences in binary output.
Link: https://github.com/KSPP/linux/issues/338
Signed-off-by: Gustavo A. R. Silva
---
drivers/gpu/drm/nouveau/nouveau_svm.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/nouveau_svm.c
b/drivers/gpu/drm/nouveau/nouveau_svm.c
index 186351ecf72f..00444ad82d18 100644
--- a/drivers/gpu/drm/nouveau/nouveau_svm.c
+++ b/drivers/gpu/drm/nouveau/nouveau_svm.c
@@ -67,7 +67,7 @@ struct nouveau_svm {
struct nouveau_svmm *svmm;
} **fault;
int fault_nr;
- } buffer[1];
+ } buffer[];
};
#define FAULT_ACCESS_READ 0
@@ -1063,7 +1063,7 @@ nouveau_svm_init(struct nouveau_drm *drm)
if (drm->client.device.info.family > NV_DEVICE_INFO_V0_PASCAL)
return;
- if (!(drm->svm = svm = kzalloc(sizeof(*drm->svm), GFP_KERNEL)))
+ if (!(drm->svm = svm = kzalloc(struct_size(drm->svm, buffer, 1),
GFP_KERNEL)))
return;
drm->svm->drm = drm;
--
2.34.1
[PATCH 0/2][next] nouveau/svm: Replace one-element array with flexible-array member
This small series aims to replace a one-element array with a flexible-array member in struct nouveau_svm. And, while at it, fix a checkpatch.pl error. Gustavo A. R. Silva (2): nouveau/svm: Replace one-element array with flexible-array member in struct nouveau_svm nouveau/svm: Split assignment from if conditional drivers/gpu/drm/nouveau/nouveau_svm.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) -- 2.34.1
[PATCH][next] fbdev: sh7760fb: Fix -Wimplicit-fallthrough warnings
Fix the following fallthrough warnings seen after building sh architecture with sh7763rdp_defconfig configuration: drivers/video/fbdev/sh7760fb.c: In function 'sh7760fb_get_color_info': drivers/video/fbdev/sh7760fb.c:138:23: warning: this statement may fall through [-Wimplicit-fallthrough=] 138 | lgray = 1; | ~~^~~ drivers/video/fbdev/sh7760fb.c:139:9: note: here 139 | case LDDFR_4BPP: | ^~~~ drivers/video/fbdev/sh7760fb.c:143:23: warning: this statement may fall through [-Wimplicit-fallthrough=] 143 | lgray = 1; | ~~^~~ drivers/video/fbdev/sh7760fb.c:144:9: note: here 144 | case LDDFR_8BPP: | ^~~~ Signed-off-by: Gustavo A. R. Silva --- drivers/video/fbdev/sh7760fb.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/video/fbdev/sh7760fb.c b/drivers/video/fbdev/sh7760fb.c index 768011bdb430..98c5227098a8 100644 --- a/drivers/video/fbdev/sh7760fb.c +++ b/drivers/video/fbdev/sh7760fb.c @@ -136,11 +136,13 @@ static int sh7760fb_get_color_info(struct device *dev, break; case LDDFR_4BPP_MONO: lgray = 1; + fallthrough; case LDDFR_4BPP: lbpp = 4; break; case LDDFR_6BPP_MONO: lgray = 1; + fallthrough; case LDDFR_8BPP: lbpp = 8; break; -- 2.34.1
[PATCH][next] drm/amdgpu/discovery: Replace fake flex-arrays with flexible-array members
Zero-length and one-element arrays are deprecated, and we are moving
towards adopting C99 flexible-array members, instead.
Use the DECLARE_FLEX_ARRAY() helper macro to transform zero-length
arrays in a union into flexible-array members. And replace a one-element
array with a C99 flexible-array member.
Address the following warnings found with GCC-13 and
-fstrict-flex-arrays=3 enabled:
drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c:1009:89: warning: array subscript
kk is outside array bounds of ‘uint32_t[0]’ {aka ‘unsigned int[]’}
[-Warray-bounds=]
drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c:1007:94: warning: array subscript
kk is outside array bounds of ‘uint64_t[0]’ {aka ‘long long unsigned int[]’}
[-Warray-bounds=]
drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c:1310:94: warning: array subscript
k is outside array bounds of ‘uint64_t[0]’ {aka ‘long long unsigned int[]’}
[-Warray-bounds=]
drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c:1309:57: warning: array subscript
k is outside array bounds of ‘uint32_t[0]’ {aka ‘unsigned int[]’}
[-Warray-bounds=]
This helps with the ongoing efforts to tighten the FORTIFY_SOURCE
routines on memcpy() and help us make progress towards globally
enabling -fstrict-flex-arrays=3 [1].
This results in no differences in binary output.
Link: https://github.com/KSPP/linux/issues/21
Link: https://github.com/KSPP/linux/issues/193
Link: https://github.com/KSPP/linux/issues/300
Link: https://gcc.gnu.org/pipermail/gcc-patches/2022-October/602902.html [1]
Signed-off-by: Gustavo A. R. Silva
---
drivers/gpu/drm/amd/include/discovery.h | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/amd/include/discovery.h
b/drivers/gpu/drm/amd/include/discovery.h
index 9181e57887db..f43e29722ef7 100644
--- a/drivers/gpu/drm/amd/include/discovery.h
+++ b/drivers/gpu/drm/amd/include/discovery.h
@@ -122,7 +122,7 @@ typedef struct ip_v3
uint8_t sub_revision : 4; /* HCID Sub-Revision */
uint8_t variant : 4;/* HW variant */
#endif
- uint32_t base_address[1]; /* Base Address list.
Corresponds to the num_base_address field*/
+ uint32_t base_address[];/* Base Address list.
Corresponds to the num_base_address field*/
} ip_v3;
typedef struct ip_v4 {
@@ -140,8 +140,8 @@ typedef struct ip_v4 {
uint8_t sub_revision : 4; /* HCID Sub-Revision */
#endif
union {
- uint32_t base_address[0]; /* 32-bit Base Address
list. Corresponds to the num_base_address field*/
- uint64_t base_address_64[0];/* 64-bit Base Address
list. Corresponds to the num_base_address field*/
+ DECLARE_FLEX_ARRAY(uint32_t, base_address); /* 32-bit Base
Address list. Corresponds to the num_base_address field*/
+ DECLARE_FLEX_ARRAY(uint64_t, base_address_64); /* 64-bit Base
Address list. Corresponds to the num_base_address field*/
} __packed;
} ip_v4;
--
2.34.1
Re: [PATCH][next] drm/i915/uapi: Replace fake flex-array with flexible-array member
On 3/31/23 01:02, Jani Nikula wrote: On Thu, 30 Mar 2023, "Gustavo A. R. Silva" wrote: Friendly ping: who can take this, please? It's in drm-intel-gt-next. Awesome. :) Thank you! -- Gustavo commit 02abecdeebfcd3848b26b70778dd7f6eb0db65e1 Author: Gustavo A. R. Silva AuthorDate: Fri Mar 17 12:18:01 2023 -0600 Commit: Tvrtko Ursulin CommitDate: Tue Mar 21 08:41:18 2023 + drm/i915/uapi: Replace fake flex-array with flexible-array member
Re: [PATCH][next] drm/i915/uapi: Replace fake flex-array with flexible-array member
Hi all,
Friendly ping: who can take this, please?
Thanks
--
Gustavo
On 3/17/23 12:18, Gustavo A. R. Silva wrote:
Zero-length arrays as fake flexible arrays are deprecated and we are
moving towards adopting C99 flexible-array members instead.
Address the following warning found with GCC-13 and
-fstrict-flex-arrays=3 enabled:
drivers/gpu/drm/i915/gem/i915_gem_context.c: In function
‘set_proto_ctx_engines.isra’:
drivers/gpu/drm/i915/gem/i915_gem_context.c:769:41: warning: array subscript n
is outside array bounds of ‘struct i915_engine_class_instance[0]’
[-Warray-bounds=]
769 | if (copy_from_user(, >engines[n],
sizeof(ci))) {
| ^
./include/uapi/drm/i915_drm.h:2494:43: note: while referencing ‘engines’
2494 | struct i915_engine_class_instance engines[0];
This helps with the ongoing efforts to tighten the FORTIFY_SOURCE
routines on memcpy() and help us make progress towards globally
enabling -fstrict-flex-arrays=3 [1].
Link: https://github.com/KSPP/linux/issues/21
Link: https://github.com/KSPP/linux/issues/271
Link: https://gcc.gnu.org/pipermail/gcc-patches/2022-October/602902.html [1]
Signed-off-by: Gustavo A. R. Silva
---
include/uapi/drm/i915_drm.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
index 8df261c5ab9b..5e458d6f2895 100644
--- a/include/uapi/drm/i915_drm.h
+++ b/include/uapi/drm/i915_drm.h
@@ -2491,7 +2491,7 @@ struct i915_context_param_engines {
#define I915_CONTEXT_ENGINES_EXT_LOAD_BALANCE 0 /* see
i915_context_engines_load_balance */
#define I915_CONTEXT_ENGINES_EXT_BOND 1 /* see i915_context_engines_bond */
#define I915_CONTEXT_ENGINES_EXT_PARALLEL_SUBMIT 2 /* see
i915_context_engines_parallel_submit */
- struct i915_engine_class_instance engines[0];
+ struct i915_engine_class_instance engines[];
} __attribute__((packed));
#define I915_DEFINE_CONTEXT_PARAM_ENGINES(name__, N__) struct { \
Re: [PATCH] drm/nouveau/disp: More DP_RECEIVER_CAP_SIZE array fixes
On 2/4/23 12:43, Kees Cook wrote:
More arrays (and arguments) for dcpd were set to 16, when it looks like
DP_RECEIVER_CAP_SIZE (15) should be used. Fix the remaining cases, seen
with GCC 13:
../drivers/gpu/drm/nouveau/nvif/outp.c: In function 'nvif_outp_acquire_dp':
../include/linux/fortify-string.h:57:33: warning: array subscript 'unsigned
char[16][0]' is partly outside array bounds of 'u8[15]' {aka 'unsigned
char[15]'} [-Warray-bounds=]
57 | #define __underlying_memcpy __builtin_memcpy
| ^
...
../drivers/gpu/drm/nouveau/nvif/outp.c:140:9: note: in expansion of macro
'memcpy'
140 | memcpy(args.dp.dpcd, dpcd, sizeof(args.dp.dpcd));
| ^~
../drivers/gpu/drm/nouveau/nvif/outp.c:130:49: note: object 'dpcd' of size [0,
15]
130 | nvif_outp_acquire_dp(struct nvif_outp *outp, u8
dpcd[DP_RECEIVER_CAP_SIZE],
|
~~~^~
Fixes: 813443721331 ("drm/nouveau/disp: move DP link config into acquire")
Cc: Ben Skeggs
Cc: Lyude Paul
Cc: Karol Herbst
Cc: David Airlie
Cc: Daniel Vetter
Cc: Dave Airlie
Cc: "Gustavo A. R. Silva"
Cc: dri-devel@lists.freedesktop.org
Cc: nouv...@lists.freedesktop.org
Signed-off-by: Kees Cook
Reviewed-by: Gustavo A. R. Silva
Thanks!
--
Gustavo
---
drivers/gpu/drm/nouveau/include/nvif/if0012.h| 4 +++-
drivers/gpu/drm/nouveau/nvkm/engine/disp/outp.h | 3 ++-
drivers/gpu/drm/nouveau/nvkm/engine/disp/uoutp.c | 2 +-
3 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/include/nvif/if0012.h
b/drivers/gpu/drm/nouveau/include/nvif/if0012.h
index eb99d84eb844..16d4ad5023a3 100644
--- a/drivers/gpu/drm/nouveau/include/nvif/if0012.h
+++ b/drivers/gpu/drm/nouveau/include/nvif/if0012.h
@@ -2,6 +2,8 @@
#ifndef __NVIF_IF0012_H__
#define __NVIF_IF0012_H__
+#include
+
union nvif_outp_args {
struct nvif_outp_v0 {
__u8 version;
@@ -63,7 +65,7 @@ union nvif_outp_acquire_args {
__u8 hda;
__u8 mst;
__u8 pad04[4];
- __u8 dpcd[16];
+ __u8 dpcd[DP_RECEIVER_CAP_SIZE];
} dp;
};
} v0;
diff --git a/drivers/gpu/drm/nouveau/nvkm/engine/disp/outp.h
b/drivers/gpu/drm/nouveau/nvkm/engine/disp/outp.h
index b7631c1ab242..4e7f873f66e2 100644
--- a/drivers/gpu/drm/nouveau/nvkm/engine/disp/outp.h
+++ b/drivers/gpu/drm/nouveau/nvkm/engine/disp/outp.h
@@ -3,6 +3,7 @@
#define __NVKM_DISP_OUTP_H__
#include "priv.h"
+#include
#include
#include
#include
@@ -42,7 +43,7 @@ struct nvkm_outp {
bool aux_pwr_pu;
u8 lttpr[6];
u8 lttprs;
- u8 dpcd[16];
+ u8 dpcd[DP_RECEIVER_CAP_SIZE];
struct {
int dpcd; /* -1, or index into
SUPPORTED_LINK_RATES table */
diff --git a/drivers/gpu/drm/nouveau/nvkm/engine/disp/uoutp.c
b/drivers/gpu/drm/nouveau/nvkm/engine/disp/uoutp.c
index 4f0ca709c85a..fc283a4a1522 100644
--- a/drivers/gpu/drm/nouveau/nvkm/engine/disp/uoutp.c
+++ b/drivers/gpu/drm/nouveau/nvkm/engine/disp/uoutp.c
@@ -146,7 +146,7 @@ nvkm_uoutp_mthd_release(struct nvkm_outp *outp, void *argv,
u32 argc)
}
static int
-nvkm_uoutp_mthd_acquire_dp(struct nvkm_outp *outp, u8 dpcd[16],
+nvkm_uoutp_mthd_acquire_dp(struct nvkm_outp *outp, u8
dpcd[DP_RECEIVER_CAP_SIZE],
u8 link_nr, u8 link_bw, bool hda, bool mst)
{
int ret;
[PATCH][next] drm/i915/uapi: Replace fake flex-array with flexible-array member
Zero-length arrays as fake flexible arrays are deprecated and we are
moving towards adopting C99 flexible-array members instead.
Address the following warning found with GCC-13 and
-fstrict-flex-arrays=3 enabled:
drivers/gpu/drm/i915/gem/i915_gem_context.c: In function
‘set_proto_ctx_engines.isra’:
drivers/gpu/drm/i915/gem/i915_gem_context.c:769:41: warning: array subscript n
is outside array bounds of ‘struct i915_engine_class_instance[0]’
[-Warray-bounds=]
769 | if (copy_from_user(, >engines[n], sizeof(ci)))
{
| ^
./include/uapi/drm/i915_drm.h:2494:43: note: while referencing ‘engines’
2494 | struct i915_engine_class_instance engines[0];
This helps with the ongoing efforts to tighten the FORTIFY_SOURCE
routines on memcpy() and help us make progress towards globally
enabling -fstrict-flex-arrays=3 [1].
Link: https://github.com/KSPP/linux/issues/21
Link: https://github.com/KSPP/linux/issues/271
Link: https://gcc.gnu.org/pipermail/gcc-patches/2022-October/602902.html [1]
Signed-off-by: Gustavo A. R. Silva
---
include/uapi/drm/i915_drm.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
index 8df261c5ab9b..5e458d6f2895 100644
--- a/include/uapi/drm/i915_drm.h
+++ b/include/uapi/drm/i915_drm.h
@@ -2491,7 +2491,7 @@ struct i915_context_param_engines {
#define I915_CONTEXT_ENGINES_EXT_LOAD_BALANCE 0 /* see
i915_context_engines_load_balance */
#define I915_CONTEXT_ENGINES_EXT_BOND 1 /* see i915_context_engines_bond */
#define I915_CONTEXT_ENGINES_EXT_PARALLEL_SUBMIT 2 /* see
i915_context_engines_parallel_submit */
- struct i915_engine_class_instance engines[0];
+ struct i915_engine_class_instance engines[];
} __attribute__((packed));
#define I915_DEFINE_CONTEXT_PARAM_ENGINES(name__, N__) struct { \
--
2.34.1
[PATCH][next] drm/vmwgfx: Replace one-element array with flexible-array member
One-element arrays are deprecated, and we are replacing them with flexible
array members instead. So, replace one-element array with flexible-array
member in struct vmw_view.
This helps with the ongoing efforts to tighten the FORTIFY_SOURCE
routines on memcpy() and help us make progress towards globally
enabling -fstrict-flex-arrays=3 [1].
This results in no differences in binary output.
Link: https://github.com/KSPP/linux/issues/79
Link: https://github.com/KSPP/linux/issues/254
Link: https://gcc.gnu.org/pipermail/gcc-patches/2022-October/602902.html [1]
Signed-off-by: Gustavo A. R. Silva
---
drivers/gpu/drm/vmwgfx/vmwgfx_so.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_so.c
b/drivers/gpu/drm/vmwgfx/vmwgfx_so.c
index 4ea32b01efc0..0f696ccaddc6 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_so.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_so.c
@@ -70,7 +70,7 @@ struct vmw_view {
unsigned view_id; /* Immutable */
u32 cmd_size; /* Immutable */
bool committed;/* Protected by binding_mutex */
- u32 cmd[1];/* Immutable */
+ u32 cmd[]; /* Immutable */
};
static int vmw_view_create(struct vmw_resource *res);
--
2.34.1
Re: [PATCH][next] drm/i915/guc: Replace zero-length arrays with flexible-array members
On Tue, Jan 10, 2023 at 02:28:11PM -0500, Rodrigo Vivi wrote:
>
> On Tue, Jan 10, 2023 at 10:44:53AM -0600, Gustavo A. R. Silva wrote:
> > Zero-length arrays are deprecated[1] and we are moving towards
> > adopting C99 flexible-array members, instead. So, replace zero-length
> > arrays in a couple of structures (three, actually) with flex-array
> > members.
> >
> > This helps with the ongoing efforts to tighten the FORTIFY_SOURCE
> > routines on memcpy() and help us make progress towards globally
> > enabling -fstrict-flex-arrays=3 [2].
> >
> > Link:
> > https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays
> > [1]
> > Link: https://gcc.gnu.org/pipermail/gcc-patches/2022-October/602902.html [2]
> > Link: https://github.com/KSPP/linux/issues/78
> > Signed-off-by: Gustavo A. R. Silva
>
> > ---
> > drivers/gpu/drm/i915/gt/uc/guc_capture_fwif.h | 6 +++---
> > 1 file changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/gt/uc/guc_capture_fwif.h
> > b/drivers/gpu/drm/i915/gt/uc/guc_capture_fwif.h
> > index 3624abfd22d1..9d589c28f40f 100644
> > --- a/drivers/gpu/drm/i915/gt/uc/guc_capture_fwif.h
> > +++ b/drivers/gpu/drm/i915/gt/uc/guc_capture_fwif.h
> > @@ -73,7 +73,7 @@ struct guc_debug_capture_list_header {
> >
> > struct guc_debug_capture_list {
> > struct guc_debug_capture_list_header header;
> > - struct guc_mmio_reg regs[0];
> > + struct guc_mmio_reg regs[];
> > } __packed;
> >
> > /**
> > @@ -125,7 +125,7 @@ struct guc_state_capture_header_t {
> >
> > struct guc_state_capture_t {
> > struct guc_state_capture_header_t header;
> > - struct guc_mmio_reg mmio_entries[0];
> > + struct guc_mmio_reg mmio_entries[];
> > } __packed;
> >
> > enum guc_capture_group_types {
> > @@ -145,7 +145,7 @@ struct guc_state_capture_group_header_t {
> > /* this is the top level structure where an error-capture dump starts */
> > struct guc_state_capture_group_t {
> > struct guc_state_capture_group_header_t grp_header;
> > - struct guc_state_capture_t capture_entries[0];
> > + struct guc_state_capture_t capture_entries[];
>
> Please notice we are currently using sizeof(struct ...).
Yep; I noticed that. :)
> Along with your proposed changes, shouldn't we also start using
> the struct_size() which already take the flexible array into account?
Not necessarily. In recent times, we don't include the struct_size
changes in the same patch as the flex-array transformation. That's
usually a follow-up patch.
--
Gustavo
[PATCH][next] drm/i915/guc: Replace zero-length arrays with flexible-array members
Zero-length arrays are deprecated[1] and we are moving towards
adopting C99 flexible-array members, instead. So, replace zero-length
arrays in a couple of structures (three, actually) with flex-array
members.
This helps with the ongoing efforts to tighten the FORTIFY_SOURCE
routines on memcpy() and help us make progress towards globally
enabling -fstrict-flex-arrays=3 [2].
Link:
https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays
[1]
Link: https://gcc.gnu.org/pipermail/gcc-patches/2022-October/602902.html [2]
Link: https://github.com/KSPP/linux/issues/78
Signed-off-by: Gustavo A. R. Silva
---
drivers/gpu/drm/i915/gt/uc/guc_capture_fwif.h | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/uc/guc_capture_fwif.h
b/drivers/gpu/drm/i915/gt/uc/guc_capture_fwif.h
index 3624abfd22d1..9d589c28f40f 100644
--- a/drivers/gpu/drm/i915/gt/uc/guc_capture_fwif.h
+++ b/drivers/gpu/drm/i915/gt/uc/guc_capture_fwif.h
@@ -73,7 +73,7 @@ struct guc_debug_capture_list_header {
struct guc_debug_capture_list {
struct guc_debug_capture_list_header header;
- struct guc_mmio_reg regs[0];
+ struct guc_mmio_reg regs[];
} __packed;
/**
@@ -125,7 +125,7 @@ struct guc_state_capture_header_t {
struct guc_state_capture_t {
struct guc_state_capture_header_t header;
- struct guc_mmio_reg mmio_entries[0];
+ struct guc_mmio_reg mmio_entries[];
} __packed;
enum guc_capture_group_types {
@@ -145,7 +145,7 @@ struct guc_state_capture_group_header_t {
/* this is the top level structure where an error-capture dump starts */
struct guc_state_capture_group_t {
struct guc_state_capture_group_header_t grp_header;
- struct guc_state_capture_t capture_entries[0];
+ struct guc_state_capture_t capture_entries[];
} __packed;
/**
--
2.34.1
[PATCH][next] habanalabs: Replace zero-length arrays with flexible-array members
Zero-length arrays are deprecated[1] and we are moving towards
adopting C99 flexible-array members instead. So, replace zero-length
arrays in a couple of structures with flex-array members.
This helps with the ongoing efforts to tighten the FORTIFY_SOURCE
routines on memcpy() and help us make progress towards globally
enabling -fstrict-flex-arrays=3 [2].
Link:
https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays
[1]
Link: https://gcc.gnu.org/pipermail/gcc-patches/2022-October/602902.html [2]
Link: https://github.com/KSPP/linux/issues/78
Signed-off-by: Gustavo A. R. Silva
---
drivers/accel/habanalabs/include/gaudi2/gaudi2_packets.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/accel/habanalabs/include/gaudi2/gaudi2_packets.h
b/drivers/accel/habanalabs/include/gaudi2/gaudi2_packets.h
index 8bf90fc18bf5..a812f8503f90 100644
--- a/drivers/accel/habanalabs/include/gaudi2/gaudi2_packets.h
+++ b/drivers/accel/habanalabs/include/gaudi2/gaudi2_packets.h
@@ -59,7 +59,7 @@ struct gaudi2_packet {
/* The rest of the packet data follows. Use the corresponding
* packet_XXX struct to deference the data, based on packet type
*/
- u8 contents[0];
+ u8 contents[];
};
struct packet_nop {
@@ -80,7 +80,7 @@ struct packet_wreg32 {
struct packet_wreg_bulk {
__le32 size64;
__le32 ctl;
- __le64 values[0]; /* data starts here */
+ __le64 values[]; /* data starts here */
};
struct packet_msg_long {
--
2.34.1
[PATCH][next] drm/nouveau/nvkm: Replace zero-length array with flexible-array member
Zero-length arrays are deprecated[1] and we are moving towards
adopting C99 flexible-array members instead. So, replace zero-length
array declaration in struct nvfw_hs_load_header_v2 with flex-array
member.
This helps with the ongoing efforts to tighten the FORTIFY_SOURCE
routines on memcpy() and help us make progress towards globally
enabling -fstrict-flex-arrays=3 [2].
Link:
https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays
[1]
Link: https://gcc.gnu.org/pipermail/gcc-patches/2022-October/602902.html [2]
Link: https://github.com/KSPP/linux/issues/78
Signed-off-by: Gustavo A. R. Silva
---
drivers/gpu/drm/nouveau/include/nvfw/hs.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/nouveau/include/nvfw/hs.h
b/drivers/gpu/drm/nouveau/include/nvfw/hs.h
index 8c4cd08a7b5f..8b58b668fc0c 100644
--- a/drivers/gpu/drm/nouveau/include/nvfw/hs.h
+++ b/drivers/gpu/drm/nouveau/include/nvfw/hs.h
@@ -52,7 +52,7 @@ struct nvfw_hs_load_header_v2 {
struct {
u32 offset;
u32 size;
- } app[0];
+ } app[];
};
const struct nvfw_hs_load_header_v2 *nvfw_hs_load_header_v2(struct nvkm_subdev
*, const void *);
--
2.34.1
Re: [PATCH] fbdev: Replace 0-length array with flexible array
On Thu, Jan 05, 2023 at 11:20:38AM -0800, Kees Cook wrote:
> Zero-length arrays are deprecated[1]. Replace struct aperture's "ranges"
> 0-length array with a flexible array. (How is the size of this array
> verified?) Detected with GCC 13, using -fstrict-flex-arrays=3:
>
> samples/vfio-mdev/mdpy-fb.c: In function 'mdpy_fb_probe':
> samples/vfio-mdev/mdpy-fb.c:169:32: warning: array subscript 0 is outside
> array bounds of 'struct aperture[0]' [-Warray-bounds=]
> 169 | info->apertures->ranges[0].base = info->fix.smem_start;
> | ~~~^~~
> In file included from samples/vfio-mdev/mdpy-fb.c:21:
> include/linux/fb.h:510:19: note: while referencing 'ranges'
> 510 | } ranges[0];
> | ^~
>
> [1]
> https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays
>
> Cc: Helge Deller
> Cc: "Gustavo A. R. Silva"
> Cc: linux-fb...@vger.kernel.org
> Cc: dri-devel@lists.freedesktop.org
> Signed-off-by: Kees Cook
Reviewed-by: Gustavo A. R. Silva
Thanks!
--
Gustavo
> ---
> include/linux/fb.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/include/linux/fb.h b/include/linux/fb.h
> index 96b96323e9cb..bf59d6a3590f 100644
> --- a/include/linux/fb.h
> +++ b/include/linux/fb.h
> @@ -507,7 +507,7 @@ struct fb_info {
> struct aperture {
> resource_size_t base;
> resource_size_t size;
> - } ranges[0];
> + } ranges[];
> } *apertures;
>
> bool skip_vt_switch; /* no VT switch on suspend/resume required */
> --
> 2.34.1
>
Re: [RESEND][PATCH] drm/nouveau/fb/ga102: Replace zero-length array of trailing structs with flex-array
On Tue, Jan 03, 2023 at 03:48:36PM -0800, Kees Cook wrote:
> Zero-length arrays are deprecated[1] and are being replaced with
> flexible array members in support of the ongoing efforts to tighten the
> FORTIFY_SOURCE routines on memcpy(), correctly instrument array indexing
> with UBSAN_BOUNDS, and to globally enable -fstrict-flex-arrays=3.
>
> Replace zero-length array with flexible-array member.
>
> This results in no differences in binary output.
>
> [1] https://github.com/KSPP/linux/issues/78
>
> Cc: Ben Skeggs
> Cc: Karol Herbst
> Cc: Lyude Paul
> Cc: David Airlie
> Cc: Daniel Vetter
> Cc: Gourav Samaiya
> Cc: "Gustavo A. R. Silva"
> Cc: dri-devel@lists.freedesktop.org
> Cc: nouv...@lists.freedesktop.org
> Signed-off-by: Kees Cook
Here is my RB again:
Reviewed-by: Gustavo A. R. Silva
Thanks!
--
Gustavo
> ---
> Sent before as:
> https://lore.kernel.org/all/20221118211207.never.039-k...@kernel.org/
> ---
> drivers/gpu/drm/nouveau/include/nvfw/hs.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/nouveau/include/nvfw/hs.h
> b/drivers/gpu/drm/nouveau/include/nvfw/hs.h
> index 8c4cd08a7b5f..8b58b668fc0c 100644
> --- a/drivers/gpu/drm/nouveau/include/nvfw/hs.h
> +++ b/drivers/gpu/drm/nouveau/include/nvfw/hs.h
> @@ -52,7 +52,7 @@ struct nvfw_hs_load_header_v2 {
> struct {
> u32 offset;
> u32 size;
> - } app[0];
> + } app[];
> };
>
> const struct nvfw_hs_load_header_v2 *nvfw_hs_load_header_v2(struct
> nvkm_subdev *, const void *);
> --
> 2.34.1
>
Re: [PATCH] drm/nouveau/fb/ga102: Replace zero-length array of trailing structs with flex-array
On Fri, Nov 18, 2022 at 01:12:08PM -0800, Kees Cook wrote:
> Zero-length arrays are deprecated[1] and are being replaced with
> flexible array members in support of the ongoing efforts to tighten the
> FORTIFY_SOURCE routines on memcpy(), correctly instrument array indexing
> with UBSAN_BOUNDS, and to globally enable -fstrict-flex-arrays=3.
>
> Replace zero-length array with flexible-array member.
>
> This results in no differences in binary output.
>
> [1] https://github.com/KSPP/linux/issues/78
>
> Cc: Ben Skeggs
> Cc: Karol Herbst
> Cc: Lyude Paul
> Cc: David Airlie
> Cc: Daniel Vetter
> Cc: Gourav Samaiya
> Cc: "Gustavo A. R. Silva"
> Cc: dri-devel@lists.freedesktop.org
> Cc: nouv...@lists.freedesktop.org
> Signed-off-by: Kees Cook
Reviewed-by: Gustavo A. R. Silva
Thanks!
--
Gustavo
> ---
> drivers/gpu/drm/nouveau/include/nvfw/hs.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/nouveau/include/nvfw/hs.h
> b/drivers/gpu/drm/nouveau/include/nvfw/hs.h
> index 8c4cd08a7b5f..8b58b668fc0c 100644
> --- a/drivers/gpu/drm/nouveau/include/nvfw/hs.h
> +++ b/drivers/gpu/drm/nouveau/include/nvfw/hs.h
> @@ -52,7 +52,7 @@ struct nvfw_hs_load_header_v2 {
> struct {
> u32 offset;
> u32 size;
> - } app[0];
> + } app[];
> };
>
> const struct nvfw_hs_load_header_v2 *nvfw_hs_load_header_v2(struct
> nvkm_subdev *, const void *);
> --
> 2.34.1
>
Re: [RFC] Approaches to deal with a struct with multiple fake flexible arrays members
On Wed, Nov 09, 2022 at 10:20:34PM -0500, Alex Deucher wrote:
> On Wed, Nov 9, 2022 at 8:31 PM Paulo Miguel Almeida
> wrote:
> >
> > On Wed, Nov 09, 2022 at 06:45:57PM -0600, Gustavo A. R. Silva wrote:
> > > On Wed, Nov 09, 2022 at 04:45:42PM +1300, Paulo Miguel Almeida wrote:
> > >
> > > Adding Alex, Christian and DRM lists to the thread.
> >
> > Thanks so much for your reply Gustavo
> > Yep, that's a good idea.
> >
> > >
> > > > struct _ATOM_INIT_REG_BLOCK {
> > > > USHORT usRegIndexTblSize;/* 0 2 */
> > > > USHORT usRegDataBlkSize; /* 2 2 */
> > > > ATOM_INIT_REG_INDEX_FORMAT asRegIndexBuf[1]; /* 4 3 */
> > > > ATOM_MEMORY_SETTING_DATA_BLOCK asRegDataBuf[1]; /* 7 8 */
> > >
> > > I didn't find evidence that asRegDataBuf is used anywhere in the
> > > codebase[1].
> > > ...
> > >
> > > ...
> > > As I pointed out above, I don't see asRegDataBuf[] being used in the
> > > codebase (of course it may describe firmware memory layout). Instead,
> > > there is this jump to a block of data past asRegIndexBuf[]:
> > >
> > > drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c:1444:
> > > 1444: ATOM_MEMORY_SETTING_DATA_BLOCK *reg_data =
> > > 1445: (ATOM_MEMORY_SETTING_DATA_BLOCK *)
> > > 1446: ((u8 *)reg_block + (2 * sizeof(u16)) +
> > > 1447: le16_to_cpu(reg_block->usRegIndexTblSize));
> > >
> > > So, it seems the one relevant array, from the kernel side, is
> > > asRegIndexBuf[]. I wonder if we really need asRegDataBuf[] in that
> > > structure... or if we could try modifying that struct and only have
> > > asRegIndexBuf[] as last member? and then we can transform it into a
> > > flex-array member.
> >
> > I saw that one too. That would be the way it's currently accessing
> > asRegDataBuf member as the existing struct would make asRegDataBuf[0] point
> > to some index within the asRegIndexBuf member (as you probably got it too)
> >
> > you are right... asRegDataBuff isn't used from a static analysis
> > point of view but removing it make the code a bit cryptic, right?
> >
> > That's pickle, ay? :-)
> >
> > >
> > > If for any strong reasong we cannot remove asRegDataBuf[] then I think we
> > > could give it a try and use DECLARE_FLEX_ARRAY() to declare both arrays
> > > in the structure.
> > >
> >
> > Out of curiosity, why both rather than just asRegIndexBuf?
Because if I understand the code and Alex's reply below correctly, both
arrays are being used to describe data of variable size, and both arrays
need to stay. The situation is that it'd be _strange_ to transform just
one of them into a flex-array member and not the other, and at the same
time a flex-array member may only appear as the last member of a
struct, and it's not _great_ to have more than one flex-array member
in a struct --in fact, we can't.
DECLARE_FLEX_ARRAY() was originally designed to have flex-array members
in unions. This is, we can declare multiple flex-array members as long as
they all share the same address. Unfortunately, this is not the case with
asRegIndexBuf[] and asRegDataBuf[], and I don't see[1] DECLARE_FLEX_ARRAY()
working in this case. So, nope, we cannot use it to declare both arrays
and we also cannot have a flex-array in the middle of a struct, so we
cannot use it to declare asRegIndexBuf[] alone. :/
On the other hand, I fail to see how the current state of the code is
problematic for things like -fstrict-flex-arrays, right now. asRegDataBuf[]
is not being used for anything in the kernel, and asRegIndexBuf[] is
correctly being accessed through it's only valid index zero, after which
is decays into a pointer[2].
That struct is definitely not great (I don't love it), but we might try
to explore other cases while we determine how and if we ultimately need
to modify it.
I'll open an issue for this in the bug tracker, so we keep an eye on it.
:)
> >
> > > But first, of course, Alex, Christian, it'd be really great if we can
> > > have your input and feedback. :)
>
> This header describes the layout of memory stored in the ROM on the
> GPU. It's shared between vbios and driver so some parts aren't
> necessarily directly used by the driver. As for what to do about it,
> I'm not sure. This structure stores a set of register offsets
> (asRegIndexBuf) and data values (asRegDataBuf) for specific operations
> (e.g., walk this structure and program register X with value Y. For a
> little background on atombios, it's a
Re: [RFC] Approaches to deal with a struct with multiple fake flexible arrays members
On Wed, Nov 09, 2022 at 04:45:42PM +1300, Paulo Miguel Almeida wrote:
Adding Alex, Christian and DRM lists to the thread.
> Hi KSPP community,
>
> I've been working on replacing 1-element arrays with flex array members
> on the drm/amdgpu files. I came across one insteresting case which I
> may need to pick your brains to find a solution for it.
>
> The structure below has two fake flexible arrays but I would get an
> error if I try make them both FAM. How should/could I deal with the
> asRegIndexBuf in this case? In theory, DECLARE_FLEX_ARRAY would "work"
> but that doesn't seem to be its intended usage as far I've searched.
> (unless I got it wrong, if that's the case, feel free to set me straight)
>
> Any ideas?
>
> struct _ATOM_INIT_REG_BLOCK {
> USHORT usRegIndexTblSize;/* 0 2 */
> USHORT usRegDataBlkSize; /* 2 2 */
> ATOM_INIT_REG_INDEX_FORMAT asRegIndexBuf[1]; /* 4 3 */
> ATOM_MEMORY_SETTING_DATA_BLOCK asRegDataBuf[1]; /* 7 8 */
I didn't find evidence that asRegDataBuf is used anywhere in the
codebase[1].
>
> /* size: 15, cachelines: 1, members: 4 */
> /* last cacheline: 15 bytes */
> } __attribute__((__packed__));
Alex, Christian,
It looks like this structure is only being used as a template to populate
instances of struct atom_mc_reg_table[2] and that these[3] are the only
places where asRegIndexBuf[] is being used. Apparently, this array is only
being used to retrieve it's address so that a pointer can jump[4] in chucks
of size sizeof(ATOM_INIT_REG_INDEX_FORMAT):
drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c:1461:
1461: format = (ATOM_INIT_REG_INDEX_FORMAT *)
1462: ((u8 *)format + sizeof(ATOM_INIT_REG_INDEX_FORMAT));
up to (VBIOS_MC_REGISTER_ARRAY_SIZE * sizeof(ATOM_INIT_REG_INDEX_FORMAT))[5],
As I pointed out above, I don't see asRegDataBuf[] being used in the
codebase (of course it may describe firmware memory layout). Instead,
there is this jump to a block of data past asRegIndexBuf[]:
drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c:1444:
1444: ATOM_MEMORY_SETTING_DATA_BLOCK *reg_data =
1445: (ATOM_MEMORY_SETTING_DATA_BLOCK *)
1446: ((u8 *)reg_block + (2 * sizeof(u16)) +
1447:le16_to_cpu(reg_block->usRegIndexTblSize));
So, it seems the one relevant array, from the kernel side, is
asRegIndexBuf[]. I wonder if we really need asRegDataBuf[] in that
structure... or if we could try modifying that struct and only have
asRegIndexBuf[] as last member? and then we can transform it into a
flex-array member.
If for any strong reasong we cannot remove asRegDataBuf[] then I think we
could give it a try and use DECLARE_FLEX_ARRAY() to declare both arrays
in the structure.
But first, of course, Alex, Christian, it'd be really great if we can
have your input and feedback. :)
Thanks!
--
Gustavo
[1] https://elixir.bootlin.com/linux/latest/C/ident/asRegDataBuf
[2]
https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c#L1441
[3] https://elixir.bootlin.com/linux/latest/C/ident/asRegIndexBuf
Re: [PATCH][next] treewide: uapi: Replace zero-length arrays with flexible-array members
On Mon, Jun 27, 2022 at 12:53:43PM -0700, Stephen Hemminger wrote: > Thanks this fixes warning with gcc-12 in iproute2. > In function ‘xfrm_algo_parse’, > inlined from ‘xfrm_state_modify.constprop’ at xfrm_state.c:573:5: > xfrm_state.c:162:32: warning: writing 1 byte into a region of size 0 > [-Wstringop-overflow=] > 162 | buf[j] = val; > | ~~~^ Great! This gives me hope. :) Thanks -- Gustavo
Re: [PATCH][next] treewide: uapi: Replace zero-length arrays with flexible-array members
On Tue, Jun 28, 2022 at 10:36:51AM -0300, Jason Gunthorpe wrote: > On Tue, Jun 28, 2022 at 04:21:29AM +0200, Gustavo A. R. Silva wrote: > > > > > Though maybe we could just switch off > > > > -Wgnu-variable-sized-type-not-at-end during configuration ? > > > We need to think in a different strategy. > > I think we will need to switch off the warning in userspace - this is > doable for rdma-core. > > On the other hand, if the goal is to enable the array size check > compiler warning I would suggest focusing only on those structs that > actually hit that warning in the kernel. IIRC infiniband doesn't > trigger it because it just pointer casts the flex array to some other > struct. Yep; this is actually why I reverted those changes in rdma (before sending out the patch) when 0-day reported the same problems you pointed out[1]. Also, that's the strategy I'm following right now with the one-element array into flex-array member transformations. I'm addressing those cases in which the trailing array is actually being iterated over, first. I just added the patch to my -next tree, so it can be build-tested by other people, and let's see what else is reported this week. :) -- Gustavo [1] https://lore.kernel.org/lkml/620ca2a5.nkaeidefiyoxe9%2fu%25...@intel.com/ > > It isn't actually an array it is a placeholder for a trailing > structure, so it is never indexed. > > This is also why we hit the warning because the convient way for > userspace to compose the message is to squash the header and trailer > structs together in a super struct on the stack, then invoke the > ioctl. > > Jason
Re: [PATCH][next] treewide: uapi: Replace zero-length arrays with flexible-array members
On Tue, Jun 28, 2022 at 02:58:25AM +0200, Gustavo A. R. Silva wrote:
> On Mon, Jun 27, 2022 at 09:40:52PM -0300, Jason Gunthorpe wrote:
> > On Mon, Jun 27, 2022 at 08:27:37PM +0200, Daniel Borkmann wrote:
> > > On 6/27/22 8:04 PM, Gustavo A. R. Silva wrote:
> > > > There is a regular need in the kernel to provide a way to declare
> > > > having a dynamically sized set of trailing elements in a structure.
> > > > Kernel code should always use “flexible array members”[1] for these
> > > > cases. The older style of one-element or zero-length arrays should
> > > > no longer be used[2].
> > > >
> > > > This code was transformed with the help of Coccinelle:
> > > > (linux-5.19-rc2$ spatch --jobs $(getconf _NPROCESSORS_ONLN) --sp-file
> > > > script.cocci --include-headers --dir . > output.patch)
> > > >
> > > > @@
> > > > identifier S, member, array;
> > > > type T1, T2;
> > > > @@
> > > >
> > > > struct S {
> > > >...
> > > >T1 member;
> > > >T2 array[
> > > > - 0
> > > >];
> > > > };
> > > >
> > > > -fstrict-flex-arrays=3 is coming and we need to land these changes
> > > > to prevent issues like these in the short future:
> > > >
> > > > ../fs/minix/dir.c:337:3: warning: 'strcpy' will always overflow;
> > > > destination buffer has size 0,
> > > > but the source string has length 2 (including NUL byte)
> > > > [-Wfortify-source]
> > > > strcpy(de3->name, ".");
> > > > ^
> > > >
> > > > Since these are all [0] to [] changes, the risk to UAPI is nearly zero.
> > > > If
> > > > this breaks anything, we can use a union with a new member name.
> > > >
> > > > [1] https://en.wikipedia.org/wiki/Flexible_array_member
> > > > [2]
> > > > https://www.kernel.org/doc/html/v5.16/process/deprecated.html#zero-length-and-one-element-arrays
> > > >
> > > > Link: https://github.com/KSPP/linux/issues/78
> > > > Build-tested-by:
> > > > https://lore.kernel.org/lkml/62b675ec.wkx6aoz6cbe71vtf%25...@intel.com/
> > > > Signed-off-by: Gustavo A. R. Silva
> > > > ---
> > > > Hi all!
> > > >
> > > > JFYI: I'm adding this to my -next tree. :)
> > >
> > > Fyi, this breaks BPF CI:
> > >
> > > https://github.com/kernel-patches/bpf/runs/7078719372?check_suite_focus=true
> > >
> > > [...]
> > > progs/map_ptr_kern.c:314:26: error: field 'trie_key' with variable
> > > sized type 'struct bpf_lpm_trie_key' not at the end of a struct or class
> > > is a GNU extension [-Werror,-Wgnu-variable-sized-type-not-at-end]
> > > struct bpf_lpm_trie_key trie_key;
> > > ^
> >
> > This will break the rdma-core userspace as well, with a similar
> > error:
> >
> > /usr/bin/clang-13 -DVERBS_DEBUG -Dibverbs_EXPORTS -Iinclude
> > -I/usr/include/libnl3 -I/usr/include/drm -g -O2
> > -fdebug-prefix-map=/__w/1/s=. -fstack-protector-strong -Wformat
> > -Werror=format-security -Wdate-time -D_FORTIFY_SOURCE=2
> > -Wmissing-prototypes -Wmissing-declarations -Wwrite-strings -Wformat=2
> > -Wcast-function-type -Wformat-nonliteral -Wdate-time -Wnested-externs
> > -Wshadow -Wstrict-prototypes -Wold-style-definition -Werror
> > -Wredundant-decls -g -fPIC -std=gnu11 -MD -MT
> > libibverbs/CMakeFiles/ibverbs.dir/cmd_flow.c.o -MF
> > libibverbs/CMakeFiles/ibverbs.dir/cmd_flow.c.o.d -o
> > libibverbs/CMakeFiles/ibverbs.dir/cmd_flow.c.o -c ../libibverbs/cmd_flow.c
> > In file included from ../libibverbs/cmd_flow.c:33:
> > In file included from include/infiniband/cmd_write.h:36:
> > In file included from include/infiniband/cmd_ioctl.h:41:
> > In file included from include/infiniband/verbs.h:48:
> > In file included from include/infiniband/verbs_api.h:66:
> > In file included from include/infiniband/ib_user_ioctl_verbs.h:38:
> > include/rdma/ib_user_verbs.h:436:34: error: field 'base' with variable
> > sized type 'struct ib_uverbs_create_cq_resp' not at the end of a struct or
> > class is a GNU extension [-Werror,-Wgnu-variable-sized-type-not-at-end]
> > struct ib_uverbs_create_cq_resp base;
> > ^
> > include/rdma/ib_user_verbs.h:644:34: error: field 'base' with variable
> > sized type 'struct ib_uverbs_create_qp_resp' not at the end of a struct or
> > class is a GNU extension [-Werror,-Wgnu-variable-sized-type-not-at-end]
> > struct ib_uverbs_create_qp_resp base;
> >
> > Which is why I gave up trying to change these..
> >
> > Though maybe we could just switch off -Wgnu-variable-sized-type-not-at-end
> > during configuration ?
>
> No. I think now we can easily workaround these sorts of problems with
> something like this:
>
> struct flex {
> any_type any_member;
> union {
> type array[0];
> __DECLARE_FLEX_ARRAY(type, array_flex);
> };
> };
Mmmh... nope; this doesn't work[1].
We need to think in a different strategy.
--
Gustavo
[1] https://godbolt.org/z/av79Pqbfz
Re: [PATCH][next] treewide: uapi: Replace zero-length arrays with flexible-array members
On Mon, Jun 27, 2022 at 09:40:52PM -0300, Jason Gunthorpe wrote:
> On Mon, Jun 27, 2022 at 08:27:37PM +0200, Daniel Borkmann wrote:
> > On 6/27/22 8:04 PM, Gustavo A. R. Silva wrote:
> > > There is a regular need in the kernel to provide a way to declare
> > > having a dynamically sized set of trailing elements in a structure.
> > > Kernel code should always use “flexible array members”[1] for these
> > > cases. The older style of one-element or zero-length arrays should
> > > no longer be used[2].
> > >
> > > This code was transformed with the help of Coccinelle:
> > > (linux-5.19-rc2$ spatch --jobs $(getconf _NPROCESSORS_ONLN) --sp-file
> > > script.cocci --include-headers --dir . > output.patch)
> > >
> > > @@
> > > identifier S, member, array;
> > > type T1, T2;
> > > @@
> > >
> > > struct S {
> > >...
> > >T1 member;
> > >T2 array[
> > > - 0
> > >];
> > > };
> > >
> > > -fstrict-flex-arrays=3 is coming and we need to land these changes
> > > to prevent issues like these in the short future:
> > >
> > > ../fs/minix/dir.c:337:3: warning: 'strcpy' will always overflow;
> > > destination buffer has size 0,
> > > but the source string has length 2 (including NUL byte) [-Wfortify-source]
> > > strcpy(de3->name, ".");
> > > ^
> > >
> > > Since these are all [0] to [] changes, the risk to UAPI is nearly zero. If
> > > this breaks anything, we can use a union with a new member name.
> > >
> > > [1] https://en.wikipedia.org/wiki/Flexible_array_member
> > > [2]
> > > https://www.kernel.org/doc/html/v5.16/process/deprecated.html#zero-length-and-one-element-arrays
> > >
> > > Link: https://github.com/KSPP/linux/issues/78
> > > Build-tested-by:
> > > https://lore.kernel.org/lkml/62b675ec.wkx6aoz6cbe71vtf%25...@intel.com/
> > > Signed-off-by: Gustavo A. R. Silva
> > > ---
> > > Hi all!
> > >
> > > JFYI: I'm adding this to my -next tree. :)
> >
> > Fyi, this breaks BPF CI:
> >
> > https://github.com/kernel-patches/bpf/runs/7078719372?check_suite_focus=true
> >
> > [...]
> > progs/map_ptr_kern.c:314:26: error: field 'trie_key' with variable sized
> > type 'struct bpf_lpm_trie_key' not at the end of a struct or class is a GNU
> > extension [-Werror,-Wgnu-variable-sized-type-not-at-end]
> > struct bpf_lpm_trie_key trie_key;
> > ^
>
> This will break the rdma-core userspace as well, with a similar
> error:
>
> /usr/bin/clang-13 -DVERBS_DEBUG -Dibverbs_EXPORTS -Iinclude
> -I/usr/include/libnl3 -I/usr/include/drm -g -O2 -fdebug-prefix-map=/__w/1/s=.
> -fstack-protector-strong -Wformat -Werror=format-security -Wdate-time
> -D_FORTIFY_SOURCE=2 -Wmissing-prototypes -Wmissing-declarations
> -Wwrite-strings -Wformat=2 -Wcast-function-type -Wformat-nonliteral
> -Wdate-time -Wnested-externs -Wshadow -Wstrict-prototypes
> -Wold-style-definition -Werror -Wredundant-decls -g -fPIC -std=gnu11 -MD
> -MT libibverbs/CMakeFiles/ibverbs.dir/cmd_flow.c.o -MF
> libibverbs/CMakeFiles/ibverbs.dir/cmd_flow.c.o.d -o
> libibverbs/CMakeFiles/ibverbs.dir/cmd_flow.c.o -c ../libibverbs/cmd_flow.c
> In file included from ../libibverbs/cmd_flow.c:33:
> In file included from include/infiniband/cmd_write.h:36:
> In file included from include/infiniband/cmd_ioctl.h:41:
> In file included from include/infiniband/verbs.h:48:
> In file included from include/infiniband/verbs_api.h:66:
> In file included from include/infiniband/ib_user_ioctl_verbs.h:38:
> include/rdma/ib_user_verbs.h:436:34: error: field 'base' with variable sized
> type 'struct ib_uverbs_create_cq_resp' not at the end of a struct or class is
> a GNU extension [-Werror,-Wgnu-variable-sized-type-not-at-end]
> struct ib_uverbs_create_cq_resp base;
> ^
> include/rdma/ib_user_verbs.h:644:34: error: field 'base' with variable sized
> type 'struct ib_uverbs_create_qp_resp' not at the end of a struct or class is
> a GNU extension [-Werror,-Wgnu-variable-sized-type-not-at-end]
> struct ib_uverbs_create_qp_resp base;
>
> Which is why I gave up trying to change these..
>
> Though maybe we could just switch off -Wgnu-variable-sized-type-not-at-end
> during configuration ?
No. I think now we can easily workaround these sorts of problems with
something like this:
struct flex {
any_type any_member;
union {
type array[0];
__DECLARE_FLEX_ARRAY(type, array_flex);
};
};
and use array_flex in kernel-space.
The same for the one-elment arrays in UAPI:
struct flex {
any_type any_member;
union {
type array[1];
__DECLARE_FLEX_ARRAY(type, array_flex);
};
};
I'll use the idiom above to resolve all these warnings in a follow-up
patch. :)
Thanks
--
Gustavo
Re: [PATCH][next] treewide: uapi: Replace zero-length arrays with flexible-array members
On Mon, Jun 27, 2022 at 08:27:37PM +0200, Daniel Borkmann wrote:
> On 6/27/22 8:04 PM, Gustavo A. R. Silva wrote:
> > There is a regular need in the kernel to provide a way to declare
> > having a dynamically sized set of trailing elements in a structure.
> > Kernel code should always use “flexible array members”[1] for these
> > cases. The older style of one-element or zero-length arrays should
> > no longer be used[2].
> >
> > This code was transformed with the help of Coccinelle:
> > (linux-5.19-rc2$ spatch --jobs $(getconf _NPROCESSORS_ONLN) --sp-file
> > script.cocci --include-headers --dir . > output.patch)
> >
> > @@
> > identifier S, member, array;
> > type T1, T2;
> > @@
> >
> > struct S {
> >...
> >T1 member;
> >T2 array[
> > - 0
> >];
> > };
> >
> > -fstrict-flex-arrays=3 is coming and we need to land these changes
> > to prevent issues like these in the short future:
> >
> > ../fs/minix/dir.c:337:3: warning: 'strcpy' will always overflow;
> > destination buffer has size 0,
> > but the source string has length 2 (including NUL byte) [-Wfortify-source]
> > strcpy(de3->name, ".");
> > ^
> >
> > Since these are all [0] to [] changes, the risk to UAPI is nearly zero. If
> > this breaks anything, we can use a union with a new member name.
> >
> > [1] https://en.wikipedia.org/wiki/Flexible_array_member
> > [2]
> > https://www.kernel.org/doc/html/v5.16/process/deprecated.html#zero-length-and-one-element-arrays
> >
> > Link: https://github.com/KSPP/linux/issues/78
> > Build-tested-by:
> > https://lore.kernel.org/lkml/62b675ec.wkx6aoz6cbe71vtf%25...@intel.com/
> > Signed-off-by: Gustavo A. R. Silva
> > ---
> > Hi all!
> >
> > JFYI: I'm adding this to my -next tree. :)
>
> Fyi, this breaks BPF CI:
Thanks for the report! It seems the 0-day robot didn't catch that one.
I'll fix it up right away. :)
--
Gustavo
>
> https://github.com/kernel-patches/bpf/runs/7078719372?check_suite_focus=true
>
> [...]
> progs/map_ptr_kern.c:314:26: error: field 'trie_key' with variable sized
> type 'struct bpf_lpm_trie_key' not at the end of a struct or class is a GNU
> extension [-Werror,-Wgnu-variable-sized-type-not-at-end]
> struct bpf_lpm_trie_key trie_key;
> ^
> 1 error generated.
> make: *** [Makefile:519:
> /tmp/runner/work/bpf/bpf/tools/testing/selftests/bpf/map_ptr_kern.o] Error 1
> make: *** Waiting for unfinished jobs
> Error: Process completed with exit code 2.
[PATCH][next] treewide: uapi: Replace zero-length arrays with flexible-array members
There is a regular need in the kernel to provide a way to declare
having a dynamically sized set of trailing elements in a structure.
Kernel code should always use “flexible array members”[1] for these
cases. The older style of one-element or zero-length arrays should
no longer be used[2].
This code was transformed with the help of Coccinelle:
(linux-5.19-rc2$ spatch --jobs $(getconf _NPROCESSORS_ONLN) --sp-file
script.cocci --include-headers --dir . > output.patch)
@@
identifier S, member, array;
type T1, T2;
@@
struct S {
...
T1 member;
T2 array[
- 0
];
};
-fstrict-flex-arrays=3 is coming and we need to land these changes
to prevent issues like these in the short future:
../fs/minix/dir.c:337:3: warning: 'strcpy' will always overflow; destination
buffer has size 0,
but the source string has length 2 (including NUL byte) [-Wfortify-source]
strcpy(de3->name, ".");
^
Since these are all [0] to [] changes, the risk to UAPI is nearly zero. If
this breaks anything, we can use a union with a new member name.
[1] https://en.wikipedia.org/wiki/Flexible_array_member
[2]
https://www.kernel.org/doc/html/v5.16/process/deprecated.html#zero-length-and-one-element-arrays
Link: https://github.com/KSPP/linux/issues/78
Build-tested-by:
https://lore.kernel.org/lkml/62b675ec.wkx6aoz6cbe71vtf%25...@intel.com/
Signed-off-by: Gustavo A. R. Silva
---
Hi all!
JFYI: I'm adding this to my -next tree. :)
arch/m68k/include/uapi/asm/bootinfo.h | 4 +-
arch/mips/include/uapi/asm/ucontext.h | 2 +-
arch/s390/include/uapi/asm/hwctrset.h | 6 +-
arch/x86/include/uapi/asm/bootparam.h | 2 +-
arch/x86/include/uapi/asm/kvm.h | 12 ++--
include/uapi/drm/i915_drm.h | 6 +-
include/uapi/linux/blkzoned.h | 2 +-
include/uapi/linux/bpf.h | 2 +-
include/uapi/linux/btrfs.h| 10 +--
include/uapi/linux/btrfs_tree.h | 2 +-
include/uapi/linux/can/bcm.h | 2 +-
include/uapi/linux/connector.h| 2 +-
include/uapi/linux/cycx_cfm.h | 2 +-
include/uapi/linux/dm-ioctl.h | 8 +--
include/uapi/linux/dm-log-userspace.h | 2 +-
include/uapi/linux/ethtool.h | 28
include/uapi/linux/fanotify.h | 2 +-
include/uapi/linux/fiemap.h | 2 +-
include/uapi/linux/firewire-cdev.h| 12 ++--
include/uapi/linux/fs.h | 2 +-
include/uapi/linux/if_alg.h | 2 +-
include/uapi/linux/if_arcnet.h| 6 +-
include/uapi/linux/if_pppox.h | 4 +-
include/uapi/linux/if_tun.h | 2 +-
include/uapi/linux/igmp.h | 6 +-
include/uapi/linux/inet_diag.h| 2 +-
include/uapi/linux/inotify.h | 2 +-
include/uapi/linux/io_uring.h | 2 +-
include/uapi/linux/ip.h | 4 +-
include/uapi/linux/ip_vs.h| 4 +-
include/uapi/linux/iso_fs.h | 4 +-
include/uapi/linux/jffs2.h| 8 +--
include/uapi/linux/kcov.h | 2 +-
include/uapi/linux/kvm.h | 8 +--
include/uapi/linux/minix_fs.h | 4 +-
include/uapi/linux/mmc/ioctl.h| 2 +-
include/uapi/linux/ndctl.h| 10 +--
include/uapi/linux/net_dropmon.h | 4 +-
include/uapi/linux/netfilter/x_tables.h | 4 +-
include/uapi/linux/netfilter_arp/arp_tables.h | 6 +-
.../uapi/linux/netfilter_bridge/ebt_among.h | 2 +-
include/uapi/linux/netfilter_ipv4/ip_tables.h | 6 +-
.../uapi/linux/netfilter_ipv6/ip6_tables.h| 4 +-
include/uapi/linux/perf_event.h | 2 +-
include/uapi/linux/pkt_cls.h | 4 +-
include/uapi/linux/raid/md_p.h| 2 +-
include/uapi/linux/random.h | 2 +-
include/uapi/linux/romfs_fs.h | 4 +-
include/uapi/linux/rtnetlink.h| 2 +-
include/uapi/linux/sctp.h | 10 +--
include/uapi/linux/seg6.h | 2 +-
include/uapi/linux/seg6_iptunnel.h| 2 +-
include/uapi/linux/stm.h | 2 +-
include/uapi/linux/target_core_user.h | 2 +-
include/uapi/linux/usb/audio.h| 2 +-
include/uapi/linux/usb/cdc.h | 6 +-
include/uapi/linux/usb/ch9.h | 2 +-
include/uapi/linux/usb/raw_gadget.h | 4 +-
include/uapi/linux/usbdevice_fs.h | 4 +-
include/uapi/linux/vhost_types.h | 4 +-
include/uapi/linux/virtio_9p.h| 2 +-
include/uapi/linux/xfrm.h | 10 +--
include/uapi/rdma/hfi/hfi1_user.h | 2 +-
include/uapi/rdma/ib_user_verbs.h
[PATCH][next] drm/i915: Fix -Wstringop-overflow warning in call to intel_read_wm_latency()
Fix the following -Wstringop-overflow warnings when building with GCC-11:
drivers/gpu/drm/i915/intel_pm.c:3106:9: warning: ‘intel_read_wm_latency’
accessing 16 bytes in a region of size 10 [-Wstringop-overflow=]
3106 | intel_read_wm_latency(dev_priv, dev_priv->wm.pri_latency);
| ^
drivers/gpu/drm/i915/intel_pm.c:3106:9: note: referencing argument 2 of type
‘u16 *’ {aka ‘short unsigned int *’}
drivers/gpu/drm/i915/intel_pm.c:2861:13: note: in a call to function
‘intel_read_wm_latency’
2861 | static void intel_read_wm_latency(struct drm_i915_private *dev_priv,
| ^
by removing the over-specified array size from the argument declarations.
It seems that this code is actually safe because the size of the
array depends on the hardware generation, and the function checks
for that.
This helps with the ongoing efforts to globally enable
-Wstringop-overflow.
Link: https://github.com/KSPP/linux/issues/181
Signed-off-by: Gustavo A. R. Silva
---
drivers/gpu/drm/i915/intel_pm.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index ee0047fdc95d..5735915facc5 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -2862,7 +2862,7 @@ static void ilk_compute_wm_level(const struct
drm_i915_private *dev_priv,
}
static void intel_read_wm_latency(struct drm_i915_private *dev_priv,
- u16 wm[8])
+ u16 wm[])
{
struct intel_uncore *uncore = _priv->uncore;
--
2.27.0
Re: [PATCH][next] drm/amd/display: Fix Wstringop-overflow warnings in dc_link_dp.c
On Thu, Mar 03, 2022 at 12:19:57PM -0600, Gustavo A. R. Silva wrote:
> On Thu, Mar 03, 2022 at 09:43:28AM -0800, Kees Cook wrote:
> > On Thu, Mar 03, 2022 at 11:25:03AM -0600, Gustavo A. R. Silva wrote:
> > > Fix the following Wstringop-overflow warnings when building with GCC-11:
> > >
> > > drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc_link_dpia.c:493:17:
> > > warning: ‘dp_decide_lane_settings’ accessing 4 bytes in a region of size
> > > 1 [-Wstringop-overflow=]
> >
> > Can you "show your work" a little more here? I don't actually see the
> > what is getting fixed:
> >
> > enum dc_lane_count {
> > ...
> > LANE_COUNT_FOUR = 4,
> > ...
> > LANE_COUNT_DP_MAX = LANE_COUNT_FOUR
> > };
> >
> > struct link_training_settings {
> > ...
> > union dpcd_training_lane dpcd_lane_settings[LANE_COUNT_DP_MAX];
> > };
> >
> > void dp_hw_to_dpcd_lane_settings(
> > ...
> > union dpcd_training_lane dpcd_lane_settings[LANE_COUNT_DP_MAX])
> > {
> > ...
> > }
> >
> > static enum link_training_result dpia_training_cr_transparent(
> > ...
> > struct link_training_settings *lt_settings)
> > {
> > ...
> > dp_decide_lane_settings(lt_settings, dpcd_lane_adjust,
> > lt_settings->hw_lane_settings,
> > lt_settings->dpcd_lane_settings);
> > ...
> > }
> >
> > Everything looks to be the correct size?
>
> Yep; this fix is similar to the one for intel_pm.c in this
>
> commit e7c6e405e171fb33990a12ecfd14e6500d9e5cf2
>
> where the array size of 8 seems to be fine for all the
> struct members related (pri_latency, spr_latency, cur_latency
> and skl_latency):
>
> drivers/gpu/drm/i915/i915_drv.h:465:struct drm_i915_private {
> ...
>
> drivers/gpu/drm/i915/i915_drv.h-739-struct {
>
> ...
> drivers/gpu/drm/i915/i915_drv.h-745-/* primary */
> drivers/gpu/drm/i915/i915_drv.h-746-u16 pri_latency[5];
> drivers/gpu/drm/i915/i915_drv.h-747-/* sprite */
> drivers/gpu/drm/i915/i915_drv.h-748-u16 spr_latency[5];
> drivers/gpu/drm/i915/i915_drv.h-749-/* cursor */
> drivers/gpu/drm/i915/i915_drv.h-750-u16 cur_latency[5];
> drivers/gpu/drm/i915/i915_drv.h-751-/*
> drivers/gpu/drm/i915/i915_drv.h-752- * Raw watermark memory
> latency values
> drivers/gpu/drm/i915/i915_drv.h-753- * for SKL for all 8 levels
> drivers/gpu/drm/i915/i915_drv.h-754- * in 1us units.
> drivers/gpu/drm/i915/i915_drv.h-755- */
> drivers/gpu/drm/i915/i915_drv.h-756-u16 skl_latency[8];
>
> ...
> drivers/gpu/drm/i915/i915_drv.h-773-} wm;
> ...
> }
and in this case the ilk_wm_max_level() returns the right maximum size for the
corresponding 'struct wm' member:
drivers/gpu/drm/i915/intel_pm.c:2993:int ilk_wm_max_level(const struct
drm_i915_private *dev_priv)
drivers/gpu/drm/i915/intel_pm.c-2994-{
drivers/gpu/drm/i915/intel_pm.c-2995- /* how many WM levels are we expecting
*/
drivers/gpu/drm/i915/intel_pm.c-2996- if (HAS_HW_SAGV_WM(dev_priv))
drivers/gpu/drm/i915/intel_pm.c-2997- return 5;
drivers/gpu/drm/i915/intel_pm.c-2998- else if (DISPLAY_VER(dev_priv) >= 9)
drivers/gpu/drm/i915/intel_pm.c-2999- return 7;
drivers/gpu/drm/i915/intel_pm.c-3000- else if (IS_HASWELL(dev_priv) ||
IS_BROADWELL(dev_priv))
drivers/gpu/drm/i915/intel_pm.c-3001- return 4;
drivers/gpu/drm/i915/intel_pm.c-3002- else if (DISPLAY_VER(dev_priv) >= 6)
drivers/gpu/drm/i915/intel_pm.c-3003- return 3;
drivers/gpu/drm/i915/intel_pm.c-3004- else
drivers/gpu/drm/i915/intel_pm.c-3005- return 2;
drivers/gpu/drm/i915/intel_pm.c-3006-}
drivers/gpu/drm/i915/intel_pm.c:3009:static void intel_print_wm_latency(struct
drm_i915_private *dev_priv,
drivers/gpu/drm/i915/intel_pm.c-3010- const char
*name,
drivers/gpu/drm/i915/intel_pm.c-3011- const u16
wm[])
drivers/gpu/drm/i915/intel_pm.c-3012-{
drivers/gpu/drm/i915/intel_pm.c-3013- int level, max_level =
ilk_wm_max_level(dev_priv);
drivers/gpu/drm/i915/intel_pm.c-3014-
drivers/gpu/drm/i915/intel_pm.c-3015- for (level = 0; level <= max_level;
level++) {
drivers/gpu/drm/i915/intel_pm.c-3016- unsigned int latency =
wm[level];
drivers/gpu/drm/i915/intel_pm.c-3017-
...
}
still GCC warns about this with Wstringop-overread, as it is explained
in commit e7c6e405e171.
--
Gustavo
>
> however GCC warns about accessing bytes beyond the limits, and turning the
> argument declarations into pointers (removing the over-specified array
> size from the argument declaration) silence the warnings.
>
> --
> Gustavo
Re: [PATCH][next] drm/amd/display: Fix Wstringop-overflow warnings in dc_link_dp.c
On Thu, Mar 03, 2022 at 09:43:28AM -0800, Kees Cook wrote:
> On Thu, Mar 03, 2022 at 11:25:03AM -0600, Gustavo A. R. Silva wrote:
> > Fix the following Wstringop-overflow warnings when building with GCC-11:
> >
> > drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc_link_dpia.c:493:17:
> > warning: ‘dp_decide_lane_settings’ accessing 4 bytes in a region of size 1
> > [-Wstringop-overflow=]
>
> Can you "show your work" a little more here? I don't actually see the
> what is getting fixed:
>
> enum dc_lane_count {
> ...
> LANE_COUNT_FOUR = 4,
> ...
> LANE_COUNT_DP_MAX = LANE_COUNT_FOUR
> };
>
> struct link_training_settings {
> ...
> union dpcd_training_lane dpcd_lane_settings[LANE_COUNT_DP_MAX];
> };
>
> void dp_hw_to_dpcd_lane_settings(
> ...
> union dpcd_training_lane dpcd_lane_settings[LANE_COUNT_DP_MAX])
> {
> ...
> }
>
> static enum link_training_result dpia_training_cr_transparent(
> ...
> struct link_training_settings *lt_settings)
> {
> ...
> dp_decide_lane_settings(lt_settings, dpcd_lane_adjust,
> lt_settings->hw_lane_settings,
> lt_settings->dpcd_lane_settings);
> ...
> }
>
> Everything looks to be the correct size?
Yep; this fix is similar to the one for intel_pm.c in this
commit e7c6e405e171fb33990a12ecfd14e6500d9e5cf2
where the array size of 8 seems to be fine for all the
struct members related (pri_latency, spr_latency, cur_latency
and skl_latency):
drivers/gpu/drm/i915/i915_drv.h:465:struct drm_i915_private {
...
drivers/gpu/drm/i915/i915_drv.h-739-struct {
...
drivers/gpu/drm/i915/i915_drv.h-745-/* primary */
drivers/gpu/drm/i915/i915_drv.h-746-u16 pri_latency[5];
drivers/gpu/drm/i915/i915_drv.h-747-/* sprite */
drivers/gpu/drm/i915/i915_drv.h-748-u16 spr_latency[5];
drivers/gpu/drm/i915/i915_drv.h-749-/* cursor */
drivers/gpu/drm/i915/i915_drv.h-750-u16 cur_latency[5];
drivers/gpu/drm/i915/i915_drv.h-751-/*
drivers/gpu/drm/i915/i915_drv.h-752- * Raw watermark memory latency
values
drivers/gpu/drm/i915/i915_drv.h-753- * for SKL for all 8 levels
drivers/gpu/drm/i915/i915_drv.h-754- * in 1us units.
drivers/gpu/drm/i915/i915_drv.h-755- */
drivers/gpu/drm/i915/i915_drv.h-756-u16 skl_latency[8];
...
drivers/gpu/drm/i915/i915_drv.h-773-} wm;
...
}
however GCC warns about accessing bytes beyond the limits, and turning the
argument declarations into pointers (removing the over-specified array
size from the argument declaration) silence the warnings.
--
Gustavo
[PATCH][next] drm/amd/display: Fix Wstringop-overflow warnings in dc_link_dp.c
Fix the following Wstringop-overflow warnings when building with GCC-11:
drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc_link_dpia.c:493:17: warning:
‘dp_decide_lane_settings’ accessing 4 bytes in a region of size 1
[-Wstringop-overflow=]
drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc_link_dpia.c:493:17: warning:
‘dp_decide_lane_settings’ accessing 4 bytes in a region of size 1
[-Wstringop-overflow=]
drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc_link_dpia.c:493:17: warning:
‘dp_decide_lane_settings’ accessing 4 bytes in a region of size 1
[-Wstringop-overflow=]
drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc_link_dpia.c:388:17: warning:
‘dp_decide_lane_settings’ accessing 4 bytes in a region of size 1
[-Wstringop-overflow=]
drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc_link_dpia.c:388:17: warning:
‘dp_decide_lane_settings’ accessing 4 bytes in a region of size 1
[-Wstringop-overflow=]
drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc_link_dpia.c:388:17: warning:
‘dp_decide_lane_settings’ accessing 4 bytes in a region of size 1
[-Wstringop-overflow=]
drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc_link_dp.c:1491:17: warning:
‘dp_decide_lane_settings’ accessing 4 bytes in a region of size 1
[-Wstringop-overflow=]
drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc_link_dp.c:2613:25: warning:
‘dp_decide_lane_settings’ accessing 4 bytes in a region of size 1
[-Wstringop-overflow=]
drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc_link_dp.c:2613:25: warning:
‘dp_decide_lane_settings’ accessing 4 bytes in a region of size 1
[-Wstringop-overflow=]
by removing the over-specified array size from the argument declarations.
This helps with the ongoing efforts to globally enable
-Wstringop-overflow.
Link: https://github.com/KSPP/linux/issues/181
Signed-off-by: Gustavo A. R. Silva
---
drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c | 4 ++--
drivers/gpu/drm/amd/display/dc/inc/dc_link_dp.h | 4 ++--
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c
b/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c
index f11a8d97fb60..81952e07acf6 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c
@@ -823,7 +823,7 @@ bool dp_is_interlane_aligned(union
lane_align_status_updated align_status)
void dp_hw_to_dpcd_lane_settings(
const struct link_training_settings *lt_settings,
const struct dc_lane_settings
hw_lane_settings[LANE_COUNT_DP_MAX],
- union dpcd_training_lane dpcd_lane_settings[LANE_COUNT_DP_MAX])
+ union dpcd_training_lane dpcd_lane_settings[])
{
uint8_t lane = 0;
@@ -853,7 +853,7 @@ void dp_decide_lane_settings(
const struct link_training_settings *lt_settings,
const union lane_adjust ln_adjust[LANE_COUNT_DP_MAX],
struct dc_lane_settings hw_lane_settings[LANE_COUNT_DP_MAX],
- union dpcd_training_lane dpcd_lane_settings[LANE_COUNT_DP_MAX])
+ union dpcd_training_lane dpcd_lane_settings[])
{
uint32_t lane;
diff --git a/drivers/gpu/drm/amd/display/dc/inc/dc_link_dp.h
b/drivers/gpu/drm/amd/display/dc/inc/dc_link_dp.h
index ab9939db8cea..0d00da1640a7 100644
--- a/drivers/gpu/drm/amd/display/dc/inc/dc_link_dp.h
+++ b/drivers/gpu/drm/amd/display/dc/inc/dc_link_dp.h
@@ -147,12 +147,12 @@ bool dp_is_max_vs_reached(
void dp_hw_to_dpcd_lane_settings(
const struct link_training_settings *lt_settings,
const struct dc_lane_settings hw_lane_settings[LANE_COUNT_DP_MAX],
- union dpcd_training_lane dpcd_lane_settings[LANE_COUNT_DP_MAX]);
+ union dpcd_training_lane dpcd_lane_settings[]);
void dp_decide_lane_settings(
const struct link_training_settings *lt_settings,
const union lane_adjust ln_adjust[LANE_COUNT_DP_MAX],
struct dc_lane_settings hw_lane_settings[LANE_COUNT_DP_MAX],
- union dpcd_training_lane dpcd_lane_settings[LANE_COUNT_DP_MAX]);
+ union dpcd_training_lane dpcd_lane_settings[]);
uint32_t dp_translate_training_aux_read_interval(uint32_t
dpcd_aux_read_interval);
--
2.27.0
Re: [PATCH][next] treewide: Replace zero-length arrays with flexible-array members
On Wed, Feb 16, 2022 at 08:05:47PM +0100, Rafael J. Wysocki wrote: > On Tue, Feb 15, 2022 at 8:24 PM Gustavo A. R. Silva > wrote: > > Can you also send the ACPI patch separately, please? > > We would like to route it through the upstream ACPICA code base. Yeah; no problem. Thanks -- Gustavo
Re: [PATCH][next] treewide: Replace zero-length arrays with flexible-array members
On Tue, Feb 15, 2022 at 09:19:29PM +0200, Leon Romanovsky wrote: > On Tue, Feb 15, 2022 at 01:21:10PM -0600, Gustavo A. R. Silva wrote: > > On Tue, Feb 15, 2022 at 10:17:40AM -0800, Kees Cook wrote: > > > On Tue, Feb 15, 2022 at 11:47:43AM -0600, Gustavo A. R. Silva wrote: > > > > > > These all look trivially correct to me. Only two didn't have the end of > > > the struct visible in the patch, and checking those showed them to be > > > trailing members as well, so: > > > > > > Reviewed-by: Kees Cook > > > > I'll add this to my -next tree. > > I would like to ask you to send mlx5 patch separately to netdev. We are > working > to delete that file completely and prefer to avoid from unnecessary merge > conflicts. Oh OK. Sure thing; I will do so. Thanks -- Gustavo
Re: [PATCH][next] treewide: Replace zero-length arrays with flexible-array members
On Tue, Feb 15, 2022 at 10:17:40AM -0800, Kees Cook wrote:
> On Tue, Feb 15, 2022 at 11:47:43AM -0600, Gustavo A. R. Silva wrote:
> > There is a regular need in the kernel to provide a way to declare
> > having a dynamically sized set of trailing elements in a structure.
> > Kernel code should always use “flexible array members”[1] for these
> > cases. The older style of one-element or zero-length arrays should
> > no longer be used[2].
> >
> > This code was transformed with the help of Coccinelle:
> > (next-20220214$ spatch --jobs $(getconf _NPROCESSORS_ONLN) --sp-file
> > script.cocci --include-headers --dir . > output.patch)
> >
> > @@
> > identifier S, member, array;
> > type T1, T2;
> > @@
> >
> > struct S {
> > ...
> > T1 member;
> > T2 array[
> > - 0
> > ];
> > };
>
> These all look trivially correct to me. Only two didn't have the end of
> the struct visible in the patch, and checking those showed them to be
> trailing members as well, so:
>
> Reviewed-by: Kees Cook
I'll add this to my -next tree.
Thanks!
--
Gustavo
[PATCH][next] treewide: Replace zero-length arrays with flexible-array members
There is a regular need in the kernel to provide a way to declare
having a dynamically sized set of trailing elements in a structure.
Kernel code should always use “flexible array members”[1] for these
cases. The older style of one-element or zero-length arrays should
no longer be used[2].
This code was transformed with the help of Coccinelle:
(next-20220214$ spatch --jobs $(getconf _NPROCESSORS_ONLN) --sp-file
script.cocci --include-headers --dir . > output.patch)
@@
identifier S, member, array;
type T1, T2;
@@
struct S {
...
T1 member;
T2 array[
- 0
];
};
UAPI and wireless changes were intentionally excluded from this patch
and will be sent out separately.
[1] https://en.wikipedia.org/wiki/Flexible_array_member
[2]
https://www.kernel.org/doc/html/v5.16/process/deprecated.html#zero-length-and-one-element-arrays
Link: https://github.com/KSPP/linux/issues/78
Signed-off-by: Gustavo A. R. Silva
---
Hi all,
I'm expecting to carry this patch in my tree, so it'd be great to
get some Acks. And given the size of the patch, I'm only sending
this to mailing lists.
Thanks!
arch/alpha/include/asm/hwrpb.h| 2 +-
arch/ia64/include/asm/sal.h | 2 +-
arch/s390/include/asm/ccwgroup.h | 2 +-
arch/s390/include/asm/chsc.h | 2 +-
arch/s390/include/asm/eadm.h | 2 +-
arch/s390/include/asm/fcx.h | 4 ++--
arch/s390/include/asm/idals.h | 2 +-
arch/s390/include/asm/sclp.h | 2 +-
arch/s390/include/asm/sysinfo.h | 6 +++---
arch/sh/include/asm/thread_info.h | 2 +-
arch/sparc/include/asm/vio.h | 10 +-
arch/um/include/shared/net_kern.h | 2 +-
arch/x86/include/asm/microcode_amd.h | 2 +-
arch/x86/include/asm/microcode_intel.h| 4 ++--
arch/x86/include/asm/pci.h| 2 +-
arch/x86/include/asm/pci_x86.h| 2 +-
arch/xtensa/include/asm/bootparam.h | 2 +-
drivers/crypto/caam/pdb.h | 2 +-
drivers/gpu/drm/i915/gt/uc/intel_guc_ads.c| 2 +-
drivers/gpu/drm/nouveau/include/nvfw/hs.h | 2 +-
.../hwtracing/coresight/coresight-config.h| 2 +-
drivers/misc/bcm-vk/bcm_vk.h | 2 +-
.../misc/habanalabs/include/common/cpucp_if.h | 6 +++---
.../habanalabs/include/gaudi/gaudi_packets.h | 4 ++--
.../habanalabs/include/goya/goya_packets.h| 4 ++--
.../net/ethernet/freescale/enetc/enetc_hw.h | 2 +-
drivers/net/ethernet/i825xx/sun3_82586.h | 2 +-
.../net/ethernet/marvell/octeontx2/af/npc.h | 6 +++---
drivers/net/ethernet/qlogic/qed/qed_mfw_hsi.h | 2 +-
drivers/net/ethernet/ti/davinci_mdio.c| 2 +-
drivers/scsi/dpt/dpti_i2o.h | 2 +-
drivers/scsi/elx/libefc_sli/sli4.h| 20 +--
drivers/scsi/mpi3mr/mpi3mr.h | 2 +-
drivers/scsi/qla2xxx/qla_bsg.h| 4 ++--
drivers/scsi/qla2xxx/qla_def.h| 2 +-
drivers/scsi/qla2xxx/qla_edif_bsg.h | 4 ++--
drivers/scsi/qla2xxx/qla_fw.h | 2 +-
drivers/scsi/qla4xxx/ql4_fw.h | 2 +-
drivers/staging/r8188eu/include/ieee80211.h | 6 +++---
drivers/staging/r8188eu/include/rtw_cmd.h | 10 +-
drivers/staging/rtl8712/rtl871x_cmd.h | 8
drivers/staging/rtl8723bs/include/ieee80211.h | 2 +-
drivers/staging/rtl8723bs/include/rtw_cmd.h | 2 +-
.../include/linux/raspberrypi/vchiq.h | 2 +-
drivers/visorbus/vbuschannel.h| 2 +-
fs/cifs/ntlmssp.h | 2 +-
fs/ext4/fast_commit.h | 4 ++--
fs/ksmbd/ksmbd_netlink.h | 2 +-
fs/ksmbd/ntlmssp.h| 6 +++---
fs/ksmbd/smb2pdu.h| 8
fs/ksmbd/transport_rdma.c | 2 +-
fs/ksmbd/xattr.h | 2 +-
fs/xfs/scrub/attr.h | 2 +-
include/acpi/actbl2.h | 2 +-
include/asm-generic/tlb.h | 4 ++--
include/linux/greybus/greybus_manifest.h | 4 ++--
include/linux/greybus/hd.h| 2 +-
include/linux/greybus/module.h| 2 +-
include/linux/i3c/ccc.h | 6 +++---
include/linux/mlx5/mlx5_ifc_fpga.h| 2 +-
include/linux/platform_data/brcmfmac.h| 2 +-
.../linux/platform_data/cros_ec_commands.h| 2 +-
include/net/bluetooth/mgmt.h | 2 +-
include/net/ioam6.h | 2 +-
include/sound/sof/channel_map.h | 4 ++--
scripts/dtc/libfdt/fdt.h | 4 ++--
sound/soc/intel/atom/sst-mfld-dsp.h | 4 ++--
sound/soc/intel/skylake/skl-topology.h| 2 +-
tools/lib/perf/include/perf/event.h | 2 +-
69 fi
[PATCH][next] nouveau/svm: Use struct_size() helper in nouveau_pfns_map()
Make use of the struct_size() helper instead of an open-coded version, in order to avoid any potential type mistakes or integer overflows that, in the worse scenario, could lead to heap overflows. Link: https://github.com/KSPP/linux/issues/160 Signed-off-by: Gustavo A. R. Silva --- drivers/gpu/drm/nouveau/nouveau_svm.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/nouveau/nouveau_svm.c b/drivers/gpu/drm/nouveau/nouveau_svm.c index 266809e511e2..46a5a1016e37 100644 --- a/drivers/gpu/drm/nouveau/nouveau_svm.c +++ b/drivers/gpu/drm/nouveau/nouveau_svm.c @@ -925,8 +925,8 @@ nouveau_pfns_map(struct nouveau_svmm *svmm, struct mm_struct *mm, mutex_lock(>mutex); - ret = nvif_object_ioctl(>vmm->vmm.object, args, sizeof(*args) + - npages * sizeof(args->p.phys[0]), NULL); + ret = nvif_object_ioctl(>vmm->vmm.object, args, + struct_size(args, p.phys, npages), NULL); mutex_unlock(>mutex); } -- 2.27.0
[PATCH][next] drm/i915/guc: Use struct_size() helper in kmalloc()
Make use of the struct_size() helper instead of an open-coded version,
in order to avoid any potential type mistakes or integer overflows that,
in the worst scenario, could lead to heap overflows.
Also, address the following sparse warnings:
drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c:792:23: warning: using sizeof on a
flexible structure
Link: https://github.com/KSPP/linux/issues/174
Signed-off-by: Gustavo A. R. Silva
---
drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c
b/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c
index aa6dd6415202..e352a1aad228 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c
@@ -789,7 +789,7 @@ static struct ct_incoming_msg *ct_alloc_msg(u32 num_dwords)
{
struct ct_incoming_msg *msg;
- msg = kmalloc(sizeof(*msg) + sizeof(u32) * num_dwords, GFP_ATOMIC);
+ msg = kmalloc(struct_size(msg, msg, num_dwords), GFP_ATOMIC);
if (msg)
msg->size = num_dwords;
return msg;
--
2.27.0
Re: [PATCH] drm/dp: Remove common Post Cursor2 register handling
On 1/5/22 11:35, Kees Cook wrote:
> The link_status array was not large enough to read the Adjust Request
> Post Cursor2 register, so remove the common helper function to avoid
> an OOB read, found with a -Warray-bounds build:
>
> drivers/gpu/drm/drm_dp_helper.c: In function
> 'drm_dp_get_adjust_request_post_cursor':
> drivers/gpu/drm/drm_dp_helper.c:59:27: error: array subscript 10 is outside
> array bounds of 'const u8[6]' {aka 'const unsigned char[6]'}
> [-Werror=array-bounds]
>59 | return link_status[r - DP_LANE0_1_STATUS];
> |~~~^~~
> drivers/gpu/drm/drm_dp_helper.c:147:51: note: while referencing 'link_status'
> 147 | u8 drm_dp_get_adjust_request_post_cursor(const u8
> link_status[DP_LINK_STATUS_SIZE],
> |
> ~^~~~
>
> Replace the only user of the helper with an open-coded fetch and decode,
> similar to drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c.
>
> Fixes: 79465e0ffeb9 ("drm/dp: Add helper to get post-cursor adjustments")
This should be tagged for -stable:
Cc: sta...@vger.kernel.org
> Cc: Maarten Lankhorst
> Cc: Maxime Ripard
> Cc: Thomas Zimmermann
> Cc: David Airlie
> Cc: Daniel Vetter
> Cc: dri-devel@lists.freedesktop.org
> Signed-off-by: Kees Cook
Reviewed-by: Gustavo A. R. Silva
Thanks
--
Gustavo
> ---
> This is the alternative to:
> https://lore.kernel.org/lkml/20211203084354.3105253-1-keesc...@chromium.org/
> ---
> drivers/gpu/drm/drm_dp_helper.c | 10 --
> drivers/gpu/drm/tegra/dp.c | 11 ++-
> include/drm/drm_dp_helper.h | 2 --
> 3 files changed, 10 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
> index 23f9073bc473..c9528aa62c9c 100644
> --- a/drivers/gpu/drm/drm_dp_helper.c
> +++ b/drivers/gpu/drm/drm_dp_helper.c
> @@ -144,16 +144,6 @@ u8 drm_dp_get_adjust_tx_ffe_preset(const u8
> link_status[DP_LINK_STATUS_SIZE],
> }
> EXPORT_SYMBOL(drm_dp_get_adjust_tx_ffe_preset);
>
> -u8 drm_dp_get_adjust_request_post_cursor(const u8
> link_status[DP_LINK_STATUS_SIZE],
> - unsigned int lane)
> -{
> - unsigned int offset = DP_ADJUST_REQUEST_POST_CURSOR2;
> - u8 value = dp_link_status(link_status, offset);
> -
> - return (value >> (lane << 1)) & 0x3;
> -}
> -EXPORT_SYMBOL(drm_dp_get_adjust_request_post_cursor);
> -
> static int __8b10b_clock_recovery_delay_us(const struct drm_dp_aux *aux, u8
> rd_interval)
> {
> if (rd_interval > 4)
> diff --git a/drivers/gpu/drm/tegra/dp.c b/drivers/gpu/drm/tegra/dp.c
> index 70dfb7d1dec5..f5535eb04c6b 100644
> --- a/drivers/gpu/drm/tegra/dp.c
> +++ b/drivers/gpu/drm/tegra/dp.c
> @@ -549,6 +549,15 @@ static void drm_dp_link_get_adjustments(struct
> drm_dp_link *link,
> {
> struct drm_dp_link_train_set *adjust = >train.adjust;
> unsigned int i;
> + u8 post_cursor;
> + int err;
> +
> + err = drm_dp_dpcd_read(link->aux, DP_ADJUST_REQUEST_POST_CURSOR2,
> +_cursor, sizeof(post_cursor));
> + if (err < 0) {
> + DRM_ERROR("failed to read post_cursor2: %d\n", err);
> + post_cursor = 0;
> + }
>
> for (i = 0; i < link->lanes; i++) {
> adjust->voltage_swing[i] =
> @@ -560,7 +569,7 @@ static void drm_dp_link_get_adjustments(struct
> drm_dp_link *link,
> DP_TRAIN_PRE_EMPHASIS_SHIFT;
>
> adjust->post_cursor[i] =
> - drm_dp_get_adjust_request_post_cursor(status, i);
> + (post_cursor >> (i << 1)) & 0x3;
> }
> }
>
> diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
> index 472dac376284..fdf3cf6ccc02 100644
> --- a/include/drm/drm_dp_helper.h
> +++ b/include/drm/drm_dp_helper.h
> @@ -1528,8 +1528,6 @@ u8 drm_dp_get_adjust_request_pre_emphasis(const u8
> link_status[DP_LINK_STATUS_SI
> int lane);
> u8 drm_dp_get_adjust_tx_ffe_preset(const u8 link_status[DP_LINK_STATUS_SIZE],
> int lane);
> -u8 drm_dp_get_adjust_request_post_cursor(const u8
> link_status[DP_LINK_STATUS_SIZE],
> - unsigned int lane);
>
> #define DP_BRANCH_OUI_HEADER_SIZE0xc
> #define DP_RECEIVER_CAP_SIZE 0xf
>
Re: [PATCH] drm/dp: Fix off-by-one in register cache size
On Fri, Dec 03, 2021 at 12:43:33AM -0800, Kees Cook wrote:
> The pcon_dsc_dpcd array holds 13 registers (0x92 through 0x9E). Fix the
> math to calculate the max size. Found from a -Warray-bounds build:
>
> drivers/gpu/drm/drm_dp_helper.c: In function 'drm_dp_pcon_dsc_bpp_incr':
> drivers/gpu/drm/drm_dp_helper.c:3130:28: error: array subscript 12 is outside
> array bounds of 'const u8[12]' {aka 'const unsigned char[12]'}
> [-Werror=array-bounds]
> 3130 | buf = pcon_dsc_dpcd[DP_PCON_DSC_BPP_INCR -
> DP_PCON_DSC_ENCODER];
> |
> ~^~~~
> drivers/gpu/drm/drm_dp_helper.c:3126:39: note: while referencing
> 'pcon_dsc_dpcd'
> 3126 | int drm_dp_pcon_dsc_bpp_incr(const u8
> pcon_dsc_dpcd[DP_PCON_DSC_ENCODER_CAP_SIZE])
> |
> ~^~~
>
> Fixes: e2e16da398d9 ("drm/dp_helper: Add support for Configuring DSC for
> HDMI2.1 Pcon")
This should be tagged for -stable:
Cc: sta...@vger.kernel.org
> Cc: Maarten Lankhorst
> Cc: Maxime Ripard
> Cc: Thomas Zimmermann
> Cc: David Airlie
> Cc: Daniel Vetter
> Cc: dri-devel@lists.freedesktop.org
> Signed-off-by: Kees Cook
Reviewed-by: Gustavo A. R. Silva
Thanks
--
Gustavo
> ---
> include/drm/drm_dp_helper.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
> index 30359e434c3f..472dac376284 100644
> --- a/include/drm/drm_dp_helper.h
> +++ b/include/drm/drm_dp_helper.h
> @@ -456,7 +456,7 @@ struct drm_panel;
> #define DP_FEC_CAPABILITY_1 0x091 /* 2.0 */
>
> /* DP-HDMI2.1 PCON DSC ENCODER SUPPORT */
> -#define DP_PCON_DSC_ENCODER_CAP_SIZE0xC /* 0x9E - 0x92 */
> +#define DP_PCON_DSC_ENCODER_CAP_SIZE0xD /* 0x92 through 0x9E */
> #define DP_PCON_DSC_ENCODER 0x092
> # define DP_PCON_DSC_ENCODER_SUPPORTED (1 << 0)
> # define DP_PCON_DSC_PPS_ENC_OVERRIDE (1 << 1)
> --
> 2.30.2
>
Re: [PATCH] fbdev: sh7760fb: document fallthrough cases
On Mon, Nov 15, 2021 at 09:35:09AM +0100, Geert Uytterhoeven wrote:
> On Mon, Nov 15, 2021 at 7:33 AM Randy Dunlap wrote:
> > Fix fallthrough warnings in sh776fb.c:
> >
> > ../drivers/video/fbdev/sh7760fb.c: In function 'sh7760fb_get_color_info':
> > ../drivers/video/fbdev/sh7760fb.c:138:23: warning: this statement may fall
> > through [-Wimplicit-fallthrough=]
> > 138 | lgray = 1;
> > ../drivers/video/fbdev/sh7760fb.c:143:23: warning: this statement may fall
> > through [-Wimplicit-fallthrough=]
> > 143 | lgray = 1;
> >
> > Just document the current state of code execution/flow.
> >
> > Fixes: 4a25e41831ee ("video: sh7760fb: SH7760/SH7763 LCDC framebuffer
> > driver")
> > Signed-off-by: Randy Dunlap
>
> Section 30.4.4 ("Data Format") of the SH7760 Group Hardware
> Manual confirms fall-through is appropriate here (especially for
> the odd 6 bpp mode).
>
> Reviewed-by: Geert Uytterhoeven
I'm taking this in my -next tree[1].
Thanks
--
Gustavo
[1]
https://git.kernel.org/pub/scm/linux/kernel/git/gustavoars/linux.git/log/?h=for-next/kspp-misc-fixes
Re: [PATCH] fbdev: sh7760fb: document fallthrough cases
On Sun, Nov 14, 2021 at 10:32:57PM -0800, Randy Dunlap wrote:
> Fix fallthrough warnings in sh776fb.c:
>
> ../drivers/video/fbdev/sh7760fb.c: In function 'sh7760fb_get_color_info':
> ../drivers/video/fbdev/sh7760fb.c:138:23: warning: this statement may fall
> through [-Wimplicit-fallthrough=]
> 138 | lgray = 1;
> ../drivers/video/fbdev/sh7760fb.c:143:23: warning: this statement may fall
> through [-Wimplicit-fallthrough=]
> 143 | lgray = 1;
>
> Just document the current state of code execution/flow.
>
> Fixes: 4a25e41831ee ("video: sh7760fb: SH7760/SH7763 LCDC framebuffer driver")
> Signed-off-by: Randy Dunlap
> Cc: "Gustavo A. R. Silva"
> Cc: Nobuhiro Iwamatsu
> Cc: Manuel Lauss
> Cc: Yoshinori Sato
> Cc: Rich Felker
> Cc: linux...@vger.kernel.org
> Cc: linux-fb...@vger.kernel.org
> Cc: dri-devel@lists.freedesktop.org
Reviewed-by: Gustavo A. R. Silva
Thanks, Randy.
--
Gustavo
> ---
> drivers/video/fbdev/sh7760fb.c |2 ++
> 1 file changed, 2 insertions(+)
>
> --- linux-next-2022.orig/drivers/video/fbdev/sh7760fb.c
> +++ linux-next-2022/drivers/video/fbdev/sh7760fb.c
> @@ -136,11 +136,13 @@ static int sh7760fb_get_color_info(struc
> break;
> case LDDFR_4BPP_MONO:
> lgray = 1;
> + fallthrough;
> case LDDFR_4BPP:
> lbpp = 4;
> break;
> case LDDFR_6BPP_MONO:
> lgray = 1;
> + fallthrough;
> case LDDFR_8BPP:
> lbpp = 8;
> break;
Re: [PATCH][next] nouveau/svm: Use kvcalloc() instead of kvzalloc()
On Thu, Oct 21, 2021 at 10:03:19AM -0700, Kees Cook wrote: > On Wed, Sep 29, 2021 at 05:28:47AM +0200, Karol Herbst wrote: > > Lack of documentation inside Linux here is a bit annoying, but do I > > understand it correctly, that the main (and probably only) difference > > is that kvcalloc checks whether the multiplication overflows and > > returns NULL in this case? > > That's correct. :) > > > On Wed, Sep 29, 2021 at 12:21 AM Gustavo A. R. Silva > > wrote: > > > > > > Use 2-factor argument form kvcalloc() instead of kvzalloc(). > > > > > > Link: https://github.com/KSPP/linux/issues/162 > > > Signed-off-by: Gustavo A. R. Silva > > Reviewed-by: Kees Cook I'll take this in my -next tree. Thanks -- Gustavo
Re: [PATCH] video: omapfb: Fix fall-through warning for Clang
On Thu, Oct 14, 2021 at 08:26:52PM +0200, Sam Ravnborg wrote: > Hi Gustavo, > On Thu, Oct 14, 2021 at 11:53:20AM -0500, Gustavo A. R. Silva wrote: > > Fix the following fallthrough warnings: > > > > drivers/video/fbdev/omap/omapfb_main.c:1558:2: warning: unannotated > > fall-through between switch labels [-Wimplicit-fallthrough] > >case 0: > >^ > >drivers/video/fbdev/omap/omapfb_main.c:1558:2: note: insert 'break;' to > > avoid fall-through > >case 0: > >^ > >break; > >1 warning generated. > > > > This helps with the ongoing efforts to globally enable > > -Wimplicit-fallthrough for Clang. > > > > Link: https://github.com/KSPP/linux/issues/115 > > Link: https://lore.kernel.org/lkml/202110141005.hujaymei-...@intel.com/ > > Reported-by: kernel test robot > > Signed-off-by: Gustavo A. R. Silva > applied to drm-misc-next. It will show up in -next in 1-2 weeks time. Great. :) Thanks, Sam. -- Gustavo
[PATCH] video: omapfb: Fix fall-through warning for Clang
Fix the following fallthrough warnings: drivers/video/fbdev/omap/omapfb_main.c:1558:2: warning: unannotated fall-through between switch labels [-Wimplicit-fallthrough] case 0: ^ drivers/video/fbdev/omap/omapfb_main.c:1558:2: note: insert 'break;' to avoid fall-through case 0: ^ break; 1 warning generated. This helps with the ongoing efforts to globally enable -Wimplicit-fallthrough for Clang. Link: https://github.com/KSPP/linux/issues/115 Link: https://lore.kernel.org/lkml/202110141005.hujaymei-...@intel.com/ Reported-by: kernel test robot Signed-off-by: Gustavo A. R. Silva --- drivers/video/fbdev/omap/omapfb_main.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/video/fbdev/omap/omapfb_main.c b/drivers/video/fbdev/omap/omapfb_main.c index 3d090d2d9ed9..b495c09e6102 100644 --- a/drivers/video/fbdev/omap/omapfb_main.c +++ b/drivers/video/fbdev/omap/omapfb_main.c @@ -1555,6 +1555,7 @@ static void omapfb_free_resources(struct omapfb_device *fbdev, int state) case 1: dev_set_drvdata(fbdev->dev, NULL); kfree(fbdev); + break; case 0: /* nothing to free */ break; -- 2.27.0
[PATCH][next] nouveau/svm: Use kvcalloc() instead of kvzalloc()
Use 2-factor argument form kvcalloc() instead of kvzalloc(). Link: https://github.com/KSPP/linux/issues/162 Signed-off-by: Gustavo A. R. Silva --- drivers/gpu/drm/nouveau/nouveau_svm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/nouveau/nouveau_svm.c b/drivers/gpu/drm/nouveau/nouveau_svm.c index b0c3422cb01f..1a896a24288a 100644 --- a/drivers/gpu/drm/nouveau/nouveau_svm.c +++ b/drivers/gpu/drm/nouveau/nouveau_svm.c @@ -992,7 +992,7 @@ nouveau_svm_fault_buffer_ctor(struct nouveau_svm *svm, s32 oclass, int id) if (ret) return ret; - buffer->fault = kvzalloc(sizeof(*buffer->fault) * buffer->entries, GFP_KERNEL); + buffer->fault = kvcalloc(sizeof(*buffer->fault), buffer->entries, GFP_KERNEL); if (!buffer->fault) return -ENOMEM; -- 2.27.0
Re: [PATCH 19/64] ip: Use struct_group() for memcpy() regions
On 7/28/21 01:31, Gustavo A. R. Silva wrote:
>
>
> On 7/28/21 01:19, Greg Kroah-Hartman wrote:
>> On Wed, Jul 28, 2021 at 01:14:33AM -0500, Gustavo A. R. Silva wrote:
>>>
>>>
>>> On 7/28/21 00:55, Greg Kroah-Hartman wrote:
>>>> On Tue, Jul 27, 2021 at 01:58:10PM -0700, Kees Cook wrote:
>>>>> In preparation for FORTIFY_SOURCE performing compile-time and run-time
>>>>> field bounds checking for memcpy(), memmove(), and memset(), avoid
>>>>> intentionally writing across neighboring fields.
>>>>>
>>>>> Use struct_group() in struct flowi4, struct ipv4hdr, and struct ipv6hdr
>>>>> around members saddr and daddr, so they can be referenced together. This
>>>>> will allow memcpy() and sizeof() to more easily reason about sizes,
>>>>> improve readability, and avoid future warnings about writing beyond the
>>>>> end of saddr.
>>>>>
>>>>> "pahole" shows no size nor member offset changes to struct flowi4.
>>>>> "objdump -d" shows no meaningful object code changes (i.e. only source
>>>>> line number induced differences.)
>>>>>
>>>>> Note that since this is a UAPI header, struct_group() has been open
>>>>> coded.
>>>>>
>>>>> Signed-off-by: Kees Cook
>>>>> ---
>>>>> include/net/flow.h| 6 --
>>>>> include/uapi/linux/if_ether.h | 12 ++--
>>>>> include/uapi/linux/ip.h | 12 ++--
>>>>> include/uapi/linux/ipv6.h | 12 ++--
>>>>> net/core/flow_dissector.c | 10 ++
>>>>> net/ipv4/ip_output.c | 6 ++
>>>>> 6 files changed, 42 insertions(+), 16 deletions(-)
>>>>>
>>>>> diff --git a/include/net/flow.h b/include/net/flow.h
>>>>> index 6f5e70240071..f1a3b6c8eae2 100644
>>>>> --- a/include/net/flow.h
>>>>> +++ b/include/net/flow.h
>>>>> @@ -81,8 +81,10 @@ struct flowi4 {
>>>>> #define flowi4_multipath_hash__fl_common.flowic_multipath_hash
>>>>>
>>>>> /* (saddr,daddr) must be grouped, same order as in IP header */
>>>>> - __be32 saddr;
>>>>> - __be32 daddr;
>>>>> + struct_group(addrs,
>>>>> + __be32 saddr;
>>>>> + __be32 daddr;
>>>>> + );
>>>>>
>>>>> union flowi_uli uli;
>>>>> #define fl4_sportuli.ports.sport
>>>>> diff --git a/include/uapi/linux/if_ether.h b/include/uapi/linux/if_ether.h
>>>>> index a0b637911d3c..8f5667b2ea92 100644
>>>>> --- a/include/uapi/linux/if_ether.h
>>>>> +++ b/include/uapi/linux/if_ether.h
>>>>> @@ -163,8 +163,16 @@
>>>>>
>>>>> #if __UAPI_DEF_ETHHDR
>>>>> struct ethhdr {
>>>>> - unsigned char h_dest[ETH_ALEN]; /* destination eth addr */
>>>>> - unsigned char h_source[ETH_ALEN]; /* source ether addr*/
>>>>> + union {
>>>>> + struct {
>>>>> + unsigned char h_dest[ETH_ALEN]; /* destination eth
>>>>> addr */
>>>>> + unsigned char h_source[ETH_ALEN]; /* source ether addr
>>>>> */
>>>>> + };
>>>>> + struct {
>>>>> + unsigned char h_dest[ETH_ALEN]; /* destination eth
>>>>> addr */
>>>>> + unsigned char h_source[ETH_ALEN]; /* source ether addr
>>>>> */
>>>>> + } addrs;
>>>>
>>>> A union of the same fields in the same structure in the same way?
>>>>
>>>> Ah, because struct_group() can not be used here? Still feels odd to see
>>>> in a userspace-visible header.
>>>>
>>>>> + };
>>>>> __be16 h_proto;/* packet type ID field */
>>>>> } __attribute__((packed));
>>>>> #endif
>>>>> diff --git a/include/uapi/linux/ip.h b/include/uapi/linux/ip.h
>>>>> index e42d13b55cf3..33647a37e56b 100644
>>>>> --- a/include/uapi/linux/ip.h
>>>>> +++ b/include/uapi/linux/ip.h
>>>>> @@ -100,8 +100,16 @@ str
Re: [PATCH 19/64] ip: Use struct_group() for memcpy() regions
On 7/28/21 01:19, Greg Kroah-Hartman wrote:
> On Wed, Jul 28, 2021 at 01:14:33AM -0500, Gustavo A. R. Silva wrote:
>>
>>
>> On 7/28/21 00:55, Greg Kroah-Hartman wrote:
>>> On Tue, Jul 27, 2021 at 01:58:10PM -0700, Kees Cook wrote:
>>>> In preparation for FORTIFY_SOURCE performing compile-time and run-time
>>>> field bounds checking for memcpy(), memmove(), and memset(), avoid
>>>> intentionally writing across neighboring fields.
>>>>
>>>> Use struct_group() in struct flowi4, struct ipv4hdr, and struct ipv6hdr
>>>> around members saddr and daddr, so they can be referenced together. This
>>>> will allow memcpy() and sizeof() to more easily reason about sizes,
>>>> improve readability, and avoid future warnings about writing beyond the
>>>> end of saddr.
>>>>
>>>> "pahole" shows no size nor member offset changes to struct flowi4.
>>>> "objdump -d" shows no meaningful object code changes (i.e. only source
>>>> line number induced differences.)
>>>>
>>>> Note that since this is a UAPI header, struct_group() has been open
>>>> coded.
>>>>
>>>> Signed-off-by: Kees Cook
>>>> ---
>>>> include/net/flow.h| 6 --
>>>> include/uapi/linux/if_ether.h | 12 ++--
>>>> include/uapi/linux/ip.h | 12 ++--
>>>> include/uapi/linux/ipv6.h | 12 ++--
>>>> net/core/flow_dissector.c | 10 ++
>>>> net/ipv4/ip_output.c | 6 ++
>>>> 6 files changed, 42 insertions(+), 16 deletions(-)
>>>>
>>>> diff --git a/include/net/flow.h b/include/net/flow.h
>>>> index 6f5e70240071..f1a3b6c8eae2 100644
>>>> --- a/include/net/flow.h
>>>> +++ b/include/net/flow.h
>>>> @@ -81,8 +81,10 @@ struct flowi4 {
>>>> #define flowi4_multipath_hash __fl_common.flowic_multipath_hash
>>>>
>>>>/* (saddr,daddr) must be grouped, same order as in IP header */
>>>> - __be32 saddr;
>>>> - __be32 daddr;
>>>> + struct_group(addrs,
>>>> + __be32 saddr;
>>>> + __be32 daddr;
>>>> + );
>>>>
>>>>union flowi_uli uli;
>>>> #define fl4_sport uli.ports.sport
>>>> diff --git a/include/uapi/linux/if_ether.h b/include/uapi/linux/if_ether.h
>>>> index a0b637911d3c..8f5667b2ea92 100644
>>>> --- a/include/uapi/linux/if_ether.h
>>>> +++ b/include/uapi/linux/if_ether.h
>>>> @@ -163,8 +163,16 @@
>>>>
>>>> #if __UAPI_DEF_ETHHDR
>>>> struct ethhdr {
>>>> - unsigned char h_dest[ETH_ALEN]; /* destination eth addr */
>>>> - unsigned char h_source[ETH_ALEN]; /* source ether addr*/
>>>> + union {
>>>> + struct {
>>>> + unsigned char h_dest[ETH_ALEN]; /* destination eth
>>>> addr */
>>>> + unsigned char h_source[ETH_ALEN]; /* source ether addr
>>>> */
>>>> + };
>>>> + struct {
>>>> + unsigned char h_dest[ETH_ALEN]; /* destination eth
>>>> addr */
>>>> + unsigned char h_source[ETH_ALEN]; /* source ether addr
>>>> */
>>>> + } addrs;
>>>
>>> A union of the same fields in the same structure in the same way?
>>>
>>> Ah, because struct_group() can not be used here? Still feels odd to see
>>> in a userspace-visible header.
>>>
>>>> + };
>>>>__be16 h_proto;/* packet type ID field */
>>>> } __attribute__((packed));
>>>> #endif
>>>> diff --git a/include/uapi/linux/ip.h b/include/uapi/linux/ip.h
>>>> index e42d13b55cf3..33647a37e56b 100644
>>>> --- a/include/uapi/linux/ip.h
>>>> +++ b/include/uapi/linux/ip.h
>>>> @@ -100,8 +100,16 @@ struct iphdr {
>>>>__u8ttl;
>>>>__u8protocol;
>>>>__sum16 check;
>>>> - __be32 saddr;
>>>> - __be32 daddr;
>>>> + union {
>>>> + struct {
>>>> + __be32 saddr;
>>>> + __be32 daddr;
>>>> + } addrs;
>>&g
Re: [PATCH 19/64] ip: Use struct_group() for memcpy() regions
On 7/28/21 00:55, Greg Kroah-Hartman wrote:
> On Tue, Jul 27, 2021 at 01:58:10PM -0700, Kees Cook wrote:
>> In preparation for FORTIFY_SOURCE performing compile-time and run-time
>> field bounds checking for memcpy(), memmove(), and memset(), avoid
>> intentionally writing across neighboring fields.
>>
>> Use struct_group() in struct flowi4, struct ipv4hdr, and struct ipv6hdr
>> around members saddr and daddr, so they can be referenced together. This
>> will allow memcpy() and sizeof() to more easily reason about sizes,
>> improve readability, and avoid future warnings about writing beyond the
>> end of saddr.
>>
>> "pahole" shows no size nor member offset changes to struct flowi4.
>> "objdump -d" shows no meaningful object code changes (i.e. only source
>> line number induced differences.)
>>
>> Note that since this is a UAPI header, struct_group() has been open
>> coded.
>>
>> Signed-off-by: Kees Cook
>> ---
>> include/net/flow.h| 6 --
>> include/uapi/linux/if_ether.h | 12 ++--
>> include/uapi/linux/ip.h | 12 ++--
>> include/uapi/linux/ipv6.h | 12 ++--
>> net/core/flow_dissector.c | 10 ++
>> net/ipv4/ip_output.c | 6 ++
>> 6 files changed, 42 insertions(+), 16 deletions(-)
>>
>> diff --git a/include/net/flow.h b/include/net/flow.h
>> index 6f5e70240071..f1a3b6c8eae2 100644
>> --- a/include/net/flow.h
>> +++ b/include/net/flow.h
>> @@ -81,8 +81,10 @@ struct flowi4 {
>> #define flowi4_multipath_hash __fl_common.flowic_multipath_hash
>>
>> /* (saddr,daddr) must be grouped, same order as in IP header */
>> -__be32 saddr;
>> -__be32 daddr;
>> +struct_group(addrs,
>> +__be32 saddr;
>> +__be32 daddr;
>> +);
>>
>> union flowi_uli uli;
>> #define fl4_sport uli.ports.sport
>> diff --git a/include/uapi/linux/if_ether.h b/include/uapi/linux/if_ether.h
>> index a0b637911d3c..8f5667b2ea92 100644
>> --- a/include/uapi/linux/if_ether.h
>> +++ b/include/uapi/linux/if_ether.h
>> @@ -163,8 +163,16 @@
>>
>> #if __UAPI_DEF_ETHHDR
>> struct ethhdr {
>> -unsigned char h_dest[ETH_ALEN]; /* destination eth addr */
>> -unsigned char h_source[ETH_ALEN]; /* source ether addr*/
>> +union {
>> +struct {
>> +unsigned char h_dest[ETH_ALEN]; /* destination eth
>> addr */
>> +unsigned char h_source[ETH_ALEN]; /* source ether addr
>> */
>> +};
>> +struct {
>> +unsigned char h_dest[ETH_ALEN]; /* destination eth
>> addr */
>> +unsigned char h_source[ETH_ALEN]; /* source ether addr
>> */
>> +} addrs;
>
> A union of the same fields in the same structure in the same way?
>
> Ah, because struct_group() can not be used here? Still feels odd to see
> in a userspace-visible header.
>
>> +};
>> __be16 h_proto;/* packet type ID field */
>> } __attribute__((packed));
>> #endif
>> diff --git a/include/uapi/linux/ip.h b/include/uapi/linux/ip.h
>> index e42d13b55cf3..33647a37e56b 100644
>> --- a/include/uapi/linux/ip.h
>> +++ b/include/uapi/linux/ip.h
>> @@ -100,8 +100,16 @@ struct iphdr {
>> __u8ttl;
>> __u8protocol;
>> __sum16 check;
>> -__be32 saddr;
>> -__be32 daddr;
>> +union {
>> +struct {
>> +__be32 saddr;
>> +__be32 daddr;
>> +} addrs;
>> +struct {
>> +__be32 saddr;
>> +__be32 daddr;
>> +};
>
> Same here (except you named the first struct addrs, not the second,
> unlike above).
>
>
>> +};
>> /*The options start here. */
>> };
>>
>> diff --git a/include/uapi/linux/ipv6.h b/include/uapi/linux/ipv6.h
>> index b243a53fa985..1c26d32e733b 100644
>> --- a/include/uapi/linux/ipv6.h
>> +++ b/include/uapi/linux/ipv6.h
>> @@ -130,8 +130,16 @@ struct ipv6hdr {
>> __u8nexthdr;
>> __u8hop_limit;
>>
>> -struct in6_addrsaddr;
>> -struct in6_addrdaddr;
>> +union {
>> +struct {
>> +struct in6_addrsaddr;
>> +struct in6_addrdaddr;
>> +} addrs;
>> +struct {
>> +struct in6_addrsaddr;
>> +struct in6_addrdaddr;
>> +};
>
> addrs first? Consistancy is key :)
I think addrs should be second. In general, I think all newly added
non-anonymous structures should be second.
Thanks
--
Gustavo
Re: [PATCH 06/64] bnxt_en: Use struct_group_attr() for memcpy() region
On Tue, Jul 27, 2021 at 01:57:57PM -0700, Kees Cook wrote:
> In preparation for FORTIFY_SOURCE performing compile-time and run-time
> field bounds checking for memcpy(), memmove(), and memset(), avoid
> intentionally writing across neighboring fields.
>
> Use struct_group() around members queue_id, min_bw, max_bw, tsa, pri_lvl,
> and bw_weight so they can be referenced together. This will allow memcpy()
> and sizeof() to more easily reason about sizes, improve readability,
> and avoid future warnings about writing beyond the end of queue_id.
>
> "pahole" shows no size nor member offset changes to struct bnxt_cos2bw_cfg.
> "objdump -d" shows no meaningful object code changes (i.e. only source
> line number induced differences and optimizations).
>
> Signed-off-by: Kees Cook
Reviewed-by: Gustavo A. R. Silva
Thanks
--
Gustavo
> ---
> drivers/net/ethernet/broadcom/bnxt/bnxt_dcb.c | 4 ++--
> drivers/net/ethernet/broadcom/bnxt/bnxt_dcb.h | 14 --
> 2 files changed, 10 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_dcb.c
> b/drivers/net/ethernet/broadcom/bnxt/bnxt_dcb.c
> index 8a68df4d9e59..95c636f89329 100644
> --- a/drivers/net/ethernet/broadcom/bnxt/bnxt_dcb.c
> +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_dcb.c
> @@ -148,10 +148,10 @@ static int bnxt_hwrm_queue_cos2bw_qcfg(struct bnxt *bp,
> struct ieee_ets *ets)
> }
>
> data = >queue_id0 + offsetof(struct bnxt_cos2bw_cfg, queue_id);
> - for (i = 0; i < bp->max_tc; i++, data += sizeof(cos2bw) - 4) {
> + for (i = 0; i < bp->max_tc; i++, data += sizeof(cos2bw.cfg)) {
> int tc;
>
> - memcpy(_id, data, sizeof(cos2bw) - 4);
> + memcpy(, data, sizeof(cos2bw.cfg));
> if (i == 0)
> cos2bw.queue_id = resp->queue_id0;
>
> diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_dcb.h
> b/drivers/net/ethernet/broadcom/bnxt/bnxt_dcb.h
> index 6eed231de565..716742522161 100644
> --- a/drivers/net/ethernet/broadcom/bnxt/bnxt_dcb.h
> +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_dcb.h
> @@ -23,13 +23,15 @@ struct bnxt_dcb {
>
> struct bnxt_cos2bw_cfg {
> u8 pad[3];
> - u8 queue_id;
> - __le32 min_bw;
> - __le32 max_bw;
> + struct_group_attr(cfg, __packed,
> + u8 queue_id;
> + __le32 min_bw;
> + __le32 max_bw;
> #define BW_VALUE_UNIT_PERCENT1_100 (0x1UL << 29)
> - u8 tsa;
> - u8 pri_lvl;
> - u8 bw_weight;
> + u8 tsa;
> + u8 pri_lvl;
> + u8 bw_weight;
> + );
> u8 unused;
> };
>
> --
> 2.30.2
>
Re: [PATCH 05/64] skbuff: Switch structure bounds to struct_group()
On Tue, Jul 27, 2021 at 01:57:56PM -0700, Kees Cook wrote:
> In preparation for FORTIFY_SOURCE performing compile-time and run-time
> field bounds checking for memcpy(), memmove(), and memset(), avoid
> intentionally writing across neighboring fields.
>
> Replace the existing empty member position markers "headers_start" and
> "headers_end" with a struct_group(). This will allow memcpy() and sizeof()
> to more easily reason about sizes, and improve readability.
>
> "pahole" shows no size nor member offset changes to struct sk_buff.
> "objdump -d" shows no no meaningful object code changes (i.e. only source
> line number induced differences and optimizations.)
>
> Signed-off-by: Kees Cook
Reviewed-by: Gustavo A. R. Silva
Thanks
--
Gustavo
> ---
> drivers/net/wireguard/queueing.h | 4 +---
> include/linux/skbuff.h | 9 -
> net/core/skbuff.c| 14 +-
> 3 files changed, 10 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/net/wireguard/queueing.h
> b/drivers/net/wireguard/queueing.h
> index 4ef2944a68bc..52da5e963003 100644
> --- a/drivers/net/wireguard/queueing.h
> +++ b/drivers/net/wireguard/queueing.h
> @@ -79,9 +79,7 @@ static inline void wg_reset_packet(struct sk_buff *skb,
> bool encapsulating)
> u8 sw_hash = skb->sw_hash;
> u32 hash = skb->hash;
> skb_scrub_packet(skb, true);
> - memset(>headers_start, 0,
> -offsetof(struct sk_buff, headers_end) -
> -offsetof(struct sk_buff, headers_start));
> + memset(>headers, 0, sizeof(skb->headers));
> if (encapsulating) {
> skb->l4_hash = l4_hash;
> skb->sw_hash = sw_hash;
> diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
> index f19190820e63..b4032e9b130e 100644
> --- a/include/linux/skbuff.h
> +++ b/include/linux/skbuff.h
> @@ -800,11 +800,10 @@ struct sk_buff {
> __u8active_extensions;
> #endif
>
> - /* fields enclosed in headers_start/headers_end are copied
> + /* Fields enclosed in headers group are copied
>* using a single memcpy() in __copy_skb_header()
>*/
> - /* private: */
> - __u32 headers_start[0];
> + struct_group(headers,
> /* public: */
>
> /* if you move pkt_type around you also must adapt those constants */
> @@ -920,8 +919,8 @@ struct sk_buff {
> u64 kcov_handle;
> #endif
>
> - /* private: */
> - __u32 headers_end[0];
> + ); /* end headers group */
> +
> /* public: */
>
> /* These elements must be at the end, see alloc_skb() for details. */
> diff --git a/net/core/skbuff.c b/net/core/skbuff.c
> index fc7942c0dddc..5f29c65507e0 100644
> --- a/net/core/skbuff.c
> +++ b/net/core/skbuff.c
> @@ -987,12 +987,10 @@ void napi_consume_skb(struct sk_buff *skb, int budget)
> }
> EXPORT_SYMBOL(napi_consume_skb);
>
> -/* Make sure a field is enclosed inside headers_start/headers_end section */
> +/* Make sure a field is contained by headers group */
> #define CHECK_SKB_FIELD(field) \
> - BUILD_BUG_ON(offsetof(struct sk_buff, field) < \
> - offsetof(struct sk_buff, headers_start)); \
> - BUILD_BUG_ON(offsetof(struct sk_buff, field) > \
> - offsetof(struct sk_buff, headers_end));\
> + BUILD_BUG_ON(offsetof(struct sk_buff, field) != \
> + offsetof(struct sk_buff, headers.field)); \
>
> static void __copy_skb_header(struct sk_buff *new, const struct sk_buff *old)
> {
> @@ -1004,14 +1002,12 @@ static void __copy_skb_header(struct sk_buff *new,
> const struct sk_buff *old)
> __skb_ext_copy(new, old);
> __nf_copy(new, old, false);
>
> - /* Note : this field could be in headers_start/headers_end section
> + /* Note : this field could be in the headers group.
>* It is not yet because we do not want to have a 16 bit hole
>*/
> new->queue_mapping = old->queue_mapping;
>
> - memcpy(>headers_start, >headers_start,
> -offsetof(struct sk_buff, headers_end) -
> -offsetof(struct sk_buff, headers_start));
> + memcpy(>headers, >headers, sizeof(new->headers));
> CHECK_SKB_FIELD(protocol);
> CHECK_SKB_FIELD(csum);
> CHECK_SKB_FIELD(hash);
> --
> 2.30.2
>
Re: [PATCH 04/64] stddef: Introduce struct_group() helper macro
On Tue, Jul 27, 2021 at 01:57:55PM -0700, Kees Cook wrote:
> Kernel code has a regular need to describe groups of members within a
> structure usually when they need to be copied or initialized separately
> from the rest of the surrounding structure. The generally accepted design
> pattern in C is to use a named sub-struct:
>
> struct foo {
> int one;
> struct {
> int two;
> int three;
> } thing;
> int four;
> };
>
> This would allow for traditional references and sizing:
>
> memcpy(, , sizeof(dst.thing));
>
> However, doing this would mean that referencing struct members enclosed
> by such named structs would always require including the sub-struct name
> in identifiers:
>
> do_something(dst.thing.three);
>
> This has tended to be quite inflexible, especially when such groupings
> need to be added to established code which causes huge naming churn.
> Three workarounds exist in the kernel for this problem, and each have
> other negative properties.
>
> To avoid the naming churn, there is a design pattern of adding macro
> aliases for the named struct:
>
> #define f_three thing.three
>
> This ends up polluting the global namespace, and makes it difficult to
> search for identifiers.
>
> Another common work-around in kernel code avoids the pollution by avoiding
> the named struct entirely, instead identifying the group's boundaries using
> either a pair of empty anonymous structs of a pair of zero-element arrays:
>
> struct foo {
> int one;
> struct { } start;
> int two;
> int three;
> struct { } finish;
> int four;
> };
>
> struct foo {
> int one;
> int start[0];
> int two;
> int three;
> int finish[0];
> int four;
> };
>
> This allows code to avoid needing to use a sub-struct name for member
> references within the surrounding structure, but loses the benefits of
> being able to actually use such a struct, making it rather fragile. Using
> these requires open-coded calculation of sizes and offsets. The efforts
> made to avoid common mistakes include lots of comments, or adding various
> BUILD_BUG_ON()s. Such code is left with no way for the compiler to reason
> about the boundaries (e.g. the "start" object looks like it's 0 bytes
> in length and is not structurally associated with "finish"), making bounds
> checking depend on open-coded calculations:
>
> if (length > offsetof(struct foo, finish) -
>offsetof(struct foo, start))
> return -EINVAL;
> memcpy(, , length);
>
> However, the vast majority of places in the kernel that operate on
> groups of members do so without any identification of the grouping,
> relying either on comments or implicit knowledge of the struct contents,
> which is even harder for the compiler to reason about, and results in
> even more fragile manual sizing, usually depending on member locations
> outside of the region (e.g. to copy "two" and "three", use the start of
> "four" to find the size):
>
> BUILD_BUG_ON((offsetof(struct foo, four) <
> offsetof(struct foo, two)) ||
>(offsetof(struct foo, four) <
> offsetof(struct foo, three));
> if (length > offsetof(struct foo, four) -
>offsetof(struct foo, two))
> return -EINVAL;
> memcpy(, , length);
>
> And both of the prior two idioms additionally appear to write beyond the
> end of the referenced struct member, forcing the compiler to ignore any
> attempt to perform bounds checking.
>
> In order to have a regular programmatic way to describe a struct
> region that can be used for references and sizing, can be examined for
> bounds checking, avoids forcing the use of intermediate identifiers,
> and avoids polluting the global namespace, introduce the struct_group()
> macro. This macro wraps the member declarations to create an anonymous
> union of an anonymous struct (no intermediate name) and a named struct
> (for references and sizing):
>
> struct foo {
> int one;
> struct_group(thing,
> int two,
> int three,
> );
> int four;
> };
>
> if (length > sizeof(src.thing))
> return -EINVAL;
> memcpy(, , l
Re: [PATCH 03/64] rpmsg: glink: Replace strncpy() with strscpy_pad()
On Tue, Jul 27, 2021 at 01:57:54PM -0700, Kees Cook wrote:
> The use of strncpy() is considered deprecated for NUL-terminated
> strings[1]. Replace strncpy() with strscpy_pad() (as it seems this case
> expects the NUL padding to fill the allocation following the flexible
> array). This additionally silences a warning seen when building under
> -Warray-bounds:
>
> ./include/linux/fortify-string.h:38:30: warning: '__builtin_strncpy' offset
> 24 from the object at '__mptr' is out of the bounds of referenced subobject
> 'data' with type 'u8[]' {aka 'unsigned char[]'} at offset 24 [-Warray-bounds]
>38 | #define __underlying_strncpy __builtin_strncpy
> | ^
> ./include/linux/fortify-string.h:50:9: note: in expansion of macro
> '__underlying_strncpy'
>50 | return __underlying_strncpy(p, q, size);
> | ^~~~
> drivers/rpmsg/qcom_glink_native.c: In function 'qcom_glink_work':
> drivers/rpmsg/qcom_glink_native.c:36:5: note: subobject 'data' declared here
>36 | u8 data[];
> | ^~~~
>
> [1]
> https://www.kernel.org/doc/html/latest/process/deprecated.html#strncpy-on-nul-terminated-strings
>
> Signed-off-by: Kees Cook
Reviewed-by: Gustavo A. R. Silva
Thanks
--
Gustavo
> ---
> drivers/rpmsg/qcom_glink_native.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/rpmsg/qcom_glink_native.c
> b/drivers/rpmsg/qcom_glink_native.c
> index 05533c71b10e..c7b9de655080 100644
> --- a/drivers/rpmsg/qcom_glink_native.c
> +++ b/drivers/rpmsg/qcom_glink_native.c
> @@ -1440,7 +1440,7 @@ static int qcom_glink_rx_open(struct qcom_glink *glink,
> unsigned int rcid,
> }
>
> rpdev->ept = >ept;
> - strncpy(rpdev->id.name, name, RPMSG_NAME_SIZE);
> + strscpy_pad(rpdev->id.name, name, RPMSG_NAME_SIZE);
> rpdev->src = RPMSG_ADDR_ANY;
> rpdev->dst = RPMSG_ADDR_ANY;
> rpdev->ops = _device_ops;
> --
> 2.30.2
>
Re: [PATCH 01/64] media: omap3isp: Extract struct group for memcpy() region
On Tue, Jul 27, 2021 at 01:57:52PM -0700, Kees Cook wrote:
> In preparation for FORTIFY_SOURCE performing compile-time and run-time
> field bounds checking for memcpy(), memmove(), and memset(), avoid
> intentionally writing across neighboring fields. Wrap the target region
> in a common named structure. This additionally fixes a theoretical
> misalignment of the copy (since the size of "buf" changes between 64-bit
> and 32-bit, but this is likely never built for 64-bit).
>
> FWIW, I think this code is totally broken on 64-bit (which appears to
> not be a "real" build configuration): it would either always fail (with
> an uninitialized data->buf_size) or would cause corruption in userspace
> due to the copy_to_user() in the call path against an uninitialized
> data->buf value:
>
> omap3isp_stat_request_statistics_time32(...)
> struct omap3isp_stat_data data64;
> ...
> omap3isp_stat_request_statistics(stat, );
>
> int omap3isp_stat_request_statistics(struct ispstat *stat,
> struct omap3isp_stat_data *data)
> ...
> buf = isp_stat_buf_get(stat, data);
>
> static struct ispstat_buffer *isp_stat_buf_get(struct ispstat *stat,
>struct omap3isp_stat_data
> *data)
> ...
> if (buf->buf_size > data->buf_size) {
> ...
> return ERR_PTR(-EINVAL);
> }
> ...
> rval = copy_to_user(data->buf,
> buf->virt_addr,
> buf->buf_size);
>
> Regardless, additionally initialize data64 to be zero-filled to avoid
> undefined behavior.
>
> Fixes: 378e3f81cb56 ("media: omap3isp: support 64-bit version of
> omap3isp_stat_data")
> Signed-off-by: Kees Cook
> ---
> drivers/media/platform/omap3isp/ispstat.c | 5 +--
> include/uapi/linux/omap3isp.h | 44 +--
> 2 files changed, 36 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/media/platform/omap3isp/ispstat.c
> b/drivers/media/platform/omap3isp/ispstat.c
> index 5b9b57f4d9bf..ea8222fed38e 100644
> --- a/drivers/media/platform/omap3isp/ispstat.c
> +++ b/drivers/media/platform/omap3isp/ispstat.c
> @@ -512,7 +512,7 @@ int omap3isp_stat_request_statistics(struct ispstat *stat,
> int omap3isp_stat_request_statistics_time32(struct ispstat *stat,
> struct omap3isp_stat_data_time32 *data)
> {
> - struct omap3isp_stat_data data64;
> + struct omap3isp_stat_data data64 = { };
> int ret;
>
> ret = omap3isp_stat_request_statistics(stat, );
> @@ -521,7 +521,8 @@ int omap3isp_stat_request_statistics_time32(struct
> ispstat *stat,
>
> data->ts.tv_sec = data64.ts.tv_sec;
> data->ts.tv_usec = data64.ts.tv_usec;
> - memcpy(>buf, , sizeof(*data) - sizeof(data->ts));
> + data->buf = (uintptr_t)data64.buf;
> + memcpy(>frame, , sizeof(data->frame));
I think this should be:
memcpy(..., , ...);
instead.
--
Gustavo
>
> return 0;
> }
> diff --git a/include/uapi/linux/omap3isp.h b/include/uapi/linux/omap3isp.h
> index 87b55755f4ff..0a16af91621f 100644
> --- a/include/uapi/linux/omap3isp.h
> +++ b/include/uapi/linux/omap3isp.h
> @@ -159,13 +159,25 @@ struct omap3isp_h3a_aewb_config {
> };
>
> /**
> - * struct omap3isp_stat_data - Statistic data sent to or received from user
> - * @ts: Timestamp of returned framestats.
> - * @buf: Pointer to pass to user.
> + * struct omap3isp_stat_frame - Statistic data without timestamp nor pointer.
> + * @buf_size: Size of buffer.
> * @frame_number: Frame number of requested stats.
> * @cur_frame: Current frame number being processed.
> * @config_counter: Number of the configuration associated with the data.
> */
> +struct omap3isp_stat_frame {
> + __u32 buf_size;
> + __u16 frame_number;
> + __u16 cur_frame;
> + __u16 config_counter;
> +};
> +
> +/**
> + * struct omap3isp_stat_data - Statistic data sent to or received from user
> + * @ts: Timestamp of returned framestats.
> + * @buf: Pointer to pass to user.
> + * @frame: Statistic data for frame.
> + */
> struct omap3isp_stat_data {
> #ifdef __KERNEL__
> struct {
> @@ -176,10 +188,15 @@ struct omap3isp_stat_data {
> struct timeval ts;
> #endif
> void __user *buf;
> - __u32 buf_size;
> - __u16 frame_number;
> - __u16 cur_frame;
> - __u16 config_counter;
> + union {
> + struct {
> + __u32 buf_size;
> + __u16 frame_number;
> + __u16 cur_frame;
> + __u16 config_counter;
> + };
> + struct omap3isp_stat_frame frame;
> + };
> };
>
> #ifdef __KERNEL__
> @@ -189,10 +206,15 @@ struct omap3isp_stat_data_time32 {
> __s32 tv_usec;
> } ts;
> __u32 buf;
> - __u32 buf_size;
> - __u16 frame_number;
> - __u16 cur_frame;
> - __u16 config_counter;
> + union {
> +
[PATCH][next] drm/amd/display: Fix fall-through warning for Clang
In preparation to enable -Wimplicit-fallthrough for Clang, fix the following warning by replacing a /* fall through */ comment with the new pseudo-keyword macro fallthrough: rivers/gpu/drm/amd/amdgpu/../display/dc/dce/dce_aux.c:672:4: warning: unannotated fall-through between switch labels [-Wimplicit-fallthrough] case AUX_TRANSACTION_REPLY_I2C_OVER_AUX_DEFER: ^ Notice that Clang doesn't recognize /* fall through */ comments as implicit fall-through markings, so in order to globally enable -Wimplicit-fallthrough for Clang, these comments need to be replaced with fallthrough; in the whole codebase. Link: https://github.com/KSPP/linux/issues/115 Signed-off-by: Gustavo A. R. Silva --- JFYI: We had thousands of these sorts of warnings and now we are down to just 15 in linux-next. This is one of those last remaining warnings. drivers/gpu/drm/amd/display/dc/dce/dce_aux.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/amd/display/dc/dce/dce_aux.c b/drivers/gpu/drm/amd/display/dc/dce/dce_aux.c index 28631714f697..2fb88e54a4bf 100644 --- a/drivers/gpu/drm/amd/display/dc/dce/dce_aux.c +++ b/drivers/gpu/drm/amd/display/dc/dce/dce_aux.c @@ -668,7 +668,7 @@ bool dce_aux_transfer_with_retries(struct ddc_service *ddc, /* polling_timeout_period is in us */ defer_time_in_ms += aux110->polling_timeout_period / 1000; ++aux_defer_retries; - /* fall through */ + fallthrough; case AUX_TRANSACTION_REPLY_I2C_OVER_AUX_DEFER: retry_on_defer = true; fallthrough; -- 2.27.0
[PATCH][next] drm/i915/gem: Fix fall-through warning for Clang
In preparation to enable -Wimplicit-fallthrough for Clang, fix a
warning by explicitly adding a fallthrough; statement.
Link: https://github.com/KSPP/linux/issues/115
Signed-off-by: Gustavo A. R. Silva
---
JFYI: We had thousands of these sorts of warnings and now we are down
to just 13 in linux-next(20210607). This is one of those last
remaining warnings. :)
drivers/gpu/drm/i915/gem/i915_gem_shrinker.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_shrinker.c
b/drivers/gpu/drm/i915/gem/i915_gem_shrinker.c
index f4fb68e8955a..17714da24033 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_shrinker.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_shrinker.c
@@ -62,6 +62,7 @@ static void try_to_writeback(struct drm_i915_gem_object *obj,
switch (obj->mm.madv) {
case I915_MADV_DONTNEED:
i915_gem_object_truncate(obj);
+ fallthrough;
case __I915_MADV_PURGED:
return;
}
--
2.27.0
[PATCH][next] drm/amd/pm: Fix fall-through warning for Clang
In preparation to enable -Wimplicit-fallthrough for Clang, fix a warning by explicitly adding a break statement instead of letting the code fall through to the next case. Link: https://github.com/KSPP/linux/issues/115 Signed-off-by: Gustavo A. R. Silva --- JFYI: We had thousands of these sorts of warnings and now we are down to just 22 in linux-next. This is one of those last remaining warnings. :) drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu10_hwmgr.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu10_hwmgr.c b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu10_hwmgr.c index 8f71f6a4bb49..43c3f6e755e7 100644 --- a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu10_hwmgr.c +++ b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu10_hwmgr.c @@ -831,6 +831,7 @@ static int smu10_dpm_force_dpm_level(struct pp_hwmgr *hwmgr, break; case AMD_DPM_FORCED_LEVEL_MANUAL: data->fine_grain_enabled = 1; + break; case AMD_DPM_FORCED_LEVEL_PROFILE_EXIT: default: break; -- 2.27.0
Re: [PATCH RESEND][next] drm/nouveau/clk: Fix fall-through warnings for Clang
Hi all, If you don't mind, I'm taking this in my -next[1] branch for v5.14. Thanks -- Gustavo [1] https://git.kernel.org/pub/scm/linux/kernel/git/gustavoars/linux.git/log/?h=for-next/kspp On 6/1/21 17:57, Gustavo A. R. Silva wrote: > Hi, > > Friendly second ping: who can take this? > > I can add this to my -next branch for 5.14 if you don't mind. > > JFYI: We had thousands of these sorts of warnings and now we are down > to just 23 in linux-next. This is one of those last remaining warnings. > > Thanks > -- > Gustavo > > On 4/20/21 15:13, Gustavo A. R. Silva wrote: >> Hi all, >> >> Friendly ping: who can take this, please? >> >> Thanks >> -- >> Gustavo >> >> On 3/5/21 03:56, Gustavo A. R. Silva wrote: >>> In preparation to enable -Wimplicit-fallthrough for Clang, fix a warning >>> by explicitly adding a break statement instead of letting the code fall >>> through to the next case. >>> >>> Link: https://github.com/KSPP/linux/issues/115 >>> Signed-off-by: Gustavo A. R. Silva >>> --- >>> drivers/gpu/drm/nouveau/nvkm/subdev/clk/nv50.c | 1 + >>> 1 file changed, 1 insertion(+) >>> >>> diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/clk/nv50.c >>> b/drivers/gpu/drm/nouveau/nvkm/subdev/clk/nv50.c >>> index 83067763c0ec..e1d31c62f9ec 100644 >>> --- a/drivers/gpu/drm/nouveau/nvkm/subdev/clk/nv50.c >>> +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/clk/nv50.c >>> @@ -313,6 +313,7 @@ nv50_clk_read(struct nvkm_clk *base, enum nv_clk_src >>> src) >>> default: >>> break; >>> } >>> + break; >>> default: >>> break; >>> } >>>
Re: [Nouveau] [PATCH RESEND][next] drm/nouveau: Fix fall-through warnings for Clang
On 6/1/21 19:10, Karol Herbst wrote: > all three nouveau patches are > > Reviewed-by: Karol Herbst > > and I don't think anybody would mind if those get into through other > trees, but maybe drm-mist would be a good place for it if other > patches involve other drm drivers? No other person has replied after pinging multiple times and resending these patches, so I guess people don't care. I'll add this and the other similar patches to my -next branch for 5.14. Thanks, Karol. -- Gustavo > > On Wed, Jun 2, 2021 at 1:16 AM Gustavo A. R. Silva > wrote: >> >> Hi, >> >> Friendly second ping: who can take this? >> >> I can add this to my -next branch for 5.14 if you don't mind. >> >> JFYI: We had thousands of these sorts of warnings and now we are down >> to just 23 in linux-next. This is one of those last remaining warnings. >> >> Thanks >> -- >> Gustavo >> >> On 4/20/21 15:13, Gustavo A. R. Silva wrote: >>> Hi all, >>> >>> Friendly ping: who can take this, please? >>> >>> Thanks >>> -- >>> Gustavo >>> >>> On 3/5/21 03:56, Gustavo A. R. Silva wrote: >>>> In preparation to enable -Wimplicit-fallthrough for Clang, fix a couple >>>> of warnings by explicitly adding a couple of break statements instead >>>> of letting the code fall through to the next case. >>>> >>>> Link: https://github.com/KSPP/linux/issues/115 >>>> Signed-off-by: Gustavo A. R. Silva >>>> --- >>>> drivers/gpu/drm/nouveau/nouveau_bo.c| 1 + >>>> drivers/gpu/drm/nouveau/nouveau_connector.c | 1 + >>>> 2 files changed, 2 insertions(+) >>>> >>>> diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c >>>> b/drivers/gpu/drm/nouveau/nouveau_bo.c >>>> index 2375711877cf..62903c3b368d 100644 >>>> --- a/drivers/gpu/drm/nouveau/nouveau_bo.c >>>> +++ b/drivers/gpu/drm/nouveau/nouveau_bo.c >>>> @@ -443,6 +443,7 @@ nouveau_bo_pin(struct nouveau_bo *nvbo, uint32_t >>>> domain, bool contig) >>>> break; >>>> case TTM_PL_TT: >>>> error |= !(domain & NOUVEAU_GEM_DOMAIN_GART); >>>> +break; >>>> default: >>>> break; >>>> } >>>> diff --git a/drivers/gpu/drm/nouveau/nouveau_connector.c >>>> b/drivers/gpu/drm/nouveau/nouveau_connector.c >>>> index 61e6d7412505..eb844cdcaec2 100644 >>>> --- a/drivers/gpu/drm/nouveau/nouveau_connector.c >>>> +++ b/drivers/gpu/drm/nouveau/nouveau_connector.c >>>> @@ -157,6 +157,7 @@ nouveau_conn_atomic_set_property(struct drm_connector >>>> *connector, >>>> default: >>>> break; >>>> } >>>> +break; >>>> case DRM_MODE_SCALE_FULLSCREEN: >>>> case DRM_MODE_SCALE_CENTER: >>>> case DRM_MODE_SCALE_ASPECT: >>>> >> ___ >> Nouveau mailing list >> nouv...@lists.freedesktop.org >> https://lists.freedesktop.org/mailman/listinfo/nouveau >> >
Re: [PATCH RESEND][next] drm/nouveau/therm: Fix fall-through warnings for Clang
Hi all, If you don't mind, I'm taking this in my -next[1] branch for v5.14. Thanks -- Gustavo [1] https://git.kernel.org/pub/scm/linux/kernel/git/gustavoars/linux.git/log/?h=for-next/kspp On 6/1/21 17:58, Gustavo A. R. Silva wrote: > Hi, > > Friendly second ping: who can take this? > > I can add this to my -next branch for 5.14 if you don't mind. > > JFYI: We had thousands of these sorts of warnings and now we are down > to just 23 in linux-next. This is one of those last remaining warnings. > > Thanks > -- > Gustavo > > On 4/20/21 15:13, Gustavo A. R. Silva wrote: >> Hi all, >> >> Friendly ping: who can take this, please? >> >> Thanks >> -- >> Gustavo >> >> On 3/5/21 03:58, Gustavo A. R. Silva wrote: >>> In preparation to enable -Wimplicit-fallthrough for Clang, fix a warning >>> by explicitly adding a break statement instead of letting the code fall >>> through to the next case. >>> >>> Link: https://github.com/KSPP/linux/issues/115 >>> Signed-off-by: Gustavo A. R. Silva >>> --- >>> drivers/gpu/drm/nouveau/nvkm/subdev/therm/gf119.c | 1 + >>> 1 file changed, 1 insertion(+) >>> >>> diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/therm/gf119.c >>> b/drivers/gpu/drm/nouveau/nvkm/subdev/therm/gf119.c >>> index 2b031d4eaeb6..684aff7437ee 100644 >>> --- a/drivers/gpu/drm/nouveau/nvkm/subdev/therm/gf119.c >>> +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/therm/gf119.c >>> @@ -41,6 +41,7 @@ pwm_info(struct nvkm_therm *therm, int line) >>> default: >>> break; >>> } >>> + break; >>> default: >>> break; >>> } >>>
Re: [PATCH RESEND][next] drm/nouveau: Fix fall-through warnings for Clang
Hi, Friendly second ping: who can take this? I can add this to my -next branch for 5.14 if you don't mind. JFYI: We had thousands of these sorts of warnings and now we are down to just 23 in linux-next. This is one of those last remaining warnings. Thanks -- Gustavo On 4/20/21 15:13, Gustavo A. R. Silva wrote: > Hi all, > > Friendly ping: who can take this, please? > > Thanks > -- > Gustavo > > On 3/5/21 03:56, Gustavo A. R. Silva wrote: >> In preparation to enable -Wimplicit-fallthrough for Clang, fix a couple >> of warnings by explicitly adding a couple of break statements instead >> of letting the code fall through to the next case. >> >> Link: https://github.com/KSPP/linux/issues/115 >> Signed-off-by: Gustavo A. R. Silva >> --- >> drivers/gpu/drm/nouveau/nouveau_bo.c| 1 + >> drivers/gpu/drm/nouveau/nouveau_connector.c | 1 + >> 2 files changed, 2 insertions(+) >> >> diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c >> b/drivers/gpu/drm/nouveau/nouveau_bo.c >> index 2375711877cf..62903c3b368d 100644 >> --- a/drivers/gpu/drm/nouveau/nouveau_bo.c >> +++ b/drivers/gpu/drm/nouveau/nouveau_bo.c >> @@ -443,6 +443,7 @@ nouveau_bo_pin(struct nouveau_bo *nvbo, uint32_t domain, >> bool contig) >> break; >> case TTM_PL_TT: >> error |= !(domain & NOUVEAU_GEM_DOMAIN_GART); >> +break; >> default: >> break; >> } >> diff --git a/drivers/gpu/drm/nouveau/nouveau_connector.c >> b/drivers/gpu/drm/nouveau/nouveau_connector.c >> index 61e6d7412505..eb844cdcaec2 100644 >> --- a/drivers/gpu/drm/nouveau/nouveau_connector.c >> +++ b/drivers/gpu/drm/nouveau/nouveau_connector.c >> @@ -157,6 +157,7 @@ nouveau_conn_atomic_set_property(struct drm_connector >> *connector, >> default: >> break; >> } >> +break; >> case DRM_MODE_SCALE_FULLSCREEN: >> case DRM_MODE_SCALE_CENTER: >> case DRM_MODE_SCALE_ASPECT: >>
Re: [PATCH RESEND][next] drm/nouveau/clk: Fix fall-through warnings for Clang
Hi, Friendly second ping: who can take this? I can add this to my -next branch for 5.14 if you don't mind. JFYI: We had thousands of these sorts of warnings and now we are down to just 23 in linux-next. This is one of those last remaining warnings. Thanks -- Gustavo On 4/20/21 15:13, Gustavo A. R. Silva wrote: > Hi all, > > Friendly ping: who can take this, please? > > Thanks > -- > Gustavo > > On 3/5/21 03:56, Gustavo A. R. Silva wrote: >> In preparation to enable -Wimplicit-fallthrough for Clang, fix a warning >> by explicitly adding a break statement instead of letting the code fall >> through to the next case. >> >> Link: https://github.com/KSPP/linux/issues/115 >> Signed-off-by: Gustavo A. R. Silva >> --- >> drivers/gpu/drm/nouveau/nvkm/subdev/clk/nv50.c | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/clk/nv50.c >> b/drivers/gpu/drm/nouveau/nvkm/subdev/clk/nv50.c >> index 83067763c0ec..e1d31c62f9ec 100644 >> --- a/drivers/gpu/drm/nouveau/nvkm/subdev/clk/nv50.c >> +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/clk/nv50.c >> @@ -313,6 +313,7 @@ nv50_clk_read(struct nvkm_clk *base, enum nv_clk_src src) >> default: >> break; >> } >> +break; >> default: >> break; >> } >>
Re: [PATCH RESEND][next] drm/nouveau/therm: Fix fall-through warnings for Clang
Hi, Friendly second ping: who can take this? I can add this to my -next branch for 5.14 if you don't mind. JFYI: We had thousands of these sorts of warnings and now we are down to just 23 in linux-next. This is one of those last remaining warnings. Thanks -- Gustavo On 4/20/21 15:13, Gustavo A. R. Silva wrote: > Hi all, > > Friendly ping: who can take this, please? > > Thanks > -- > Gustavo > > On 3/5/21 03:58, Gustavo A. R. Silva wrote: >> In preparation to enable -Wimplicit-fallthrough for Clang, fix a warning >> by explicitly adding a break statement instead of letting the code fall >> through to the next case. >> >> Link: https://github.com/KSPP/linux/issues/115 >> Signed-off-by: Gustavo A. R. Silva >> --- >> drivers/gpu/drm/nouveau/nvkm/subdev/therm/gf119.c | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/therm/gf119.c >> b/drivers/gpu/drm/nouveau/nvkm/subdev/therm/gf119.c >> index 2b031d4eaeb6..684aff7437ee 100644 >> --- a/drivers/gpu/drm/nouveau/nvkm/subdev/therm/gf119.c >> +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/therm/gf119.c >> @@ -41,6 +41,7 @@ pwm_info(struct nvkm_therm *therm, int line) >> default: >> break; >> } >> +break; >> default: >> break; >> } >>
Re: [PATCH] drm/radeon/ni_dpm: Fix booting bug
Hi Alex,
On 5/10/21 16:17, Alex Deucher wrote:
> On Sun, May 9, 2021 at 6:48 PM Gustavo A. R. Silva
> wrote:
[..]
>>
>> Bug:
>> https://lore.kernel.org/dri-devel/3eedbe78-1fbd-4763-a7f3-ac5665e76...@xenosoft.de/
>> Fixes: 434fb1e7444a ("drm/radeon/nislands_smc.h: Replace one-element array
>> with flexible-array member in struct NISLANDS_SMC_SWSTATE")
>> Cc: sta...@vger.kernel.org
>> Reported-by: Christian Zigotzky
>> Tested-by: Christian Zigotzky
>> Link:
>> https://lore.kernel.org/dri-devel/9bb5fcbd-daf5-1669-b3e7-b8624b3c3...@xenosoft.de/
>> Signed-off-by: Gustavo A. R. Silva
>
> This seems like a lot of churn just to use flexible arrays. That
> said, if static checkers are going to keep complaining about single
> element arrays, I don't mind applying these patches since this code is
> not likely to change. Applied. Thanks.
This is not only about the one-element arrays. These fixes (together with
commits
434fb1e7444a and 96e27e8d919e) allow us to fix more than a dozen of these
out-of-bounds
warnings:
drivers/gpu/drm/radeon/ni_dpm.c:2521:20: warning: array subscript 1 is above
array bounds of ‘NISLANDS_SMC_HW_PERFORMANCE_LEVEL[1]’ {aka ‘struct
NISLANDS_SMC_HW_PERFORMANCE_LEVEL[1]’} [-Warray-bounds]
2521 | smc_state->levels[i].dpm2.MaxPS =
| ~^~~
which should be fixed in order to globally enable -Warray-bounds. :)
Thanks!
--
Gustavo
[PATCH] drm/amd/pm: Fix out-of-bounds bug
Create new structure SISLANDS_SMC_SWSTATE_SINGLE, as initialState.levels
and ACPIState.levels are never actually used as flexible arrays. Those
arrays can be used as simple objects of type
SISLANDS_SMC_HW_PERFORMANCE_LEVEL, instead.
Currently, the code fails because flexible array _levels_ in
struct SISLANDS_SMC_SWSTATE doesn't allow for code that accesses
the first element of initialState.levels and ACPIState.levels
arrays:
drivers/gpu/drm/amd/pm/powerplay/si_dpm.c:
4820: table->initialState.levels[0].mclk.vDLL_CNTL =
4821: cpu_to_be32(si_pi->clock_registers.dll_cntl);
...
5021: table->ACPIState.levels[0].mclk.vDLL_CNTL =
5022: cpu_to_be32(dll_cntl);
because such element cannot be accessed without previously allocating
enough dynamic memory for it to exist (which never actually happens).
So, there is an out-of-bounds bug in this case.
That's why struct SISLANDS_SMC_SWSTATE should only be used as type
for object driverState and new struct SISLANDS_SMC_SWSTATE_SINGLE is
created as type for objects initialState, ACPIState and ULVState.
Also, with the change from one-element array to flexible-array member
in commit 0e1aa13ca3ff ("drm/amd/pm: Replace one-element array with
flexible-array in struct SISLANDS_SMC_SWSTATE"), the size of
dpmLevels in struct SISLANDS_SMC_STATETABLE should be fixed to be
SISLANDS_MAX_SMC_PERFORMANCE_LEVELS_PER_SWSTATE instead of
SISLANDS_MAX_SMC_PERFORMANCE_LEVELS_PER_SWSTATE - 1.
Fixes: 0e1aa13ca3ff ("drm/amd/pm: Replace one-element array with flexible-array
in struct SISLANDS_SMC_SWSTATE")
Cc: sta...@vger.kernel.org
Signed-off-by: Gustavo A. R. Silva
---
drivers/gpu/drm/amd/pm/powerplay/si_dpm.c | 174 +-
.../gpu/drm/amd/pm/powerplay/sislands_smc.h | 34 ++--
2 files changed, 109 insertions(+), 99 deletions(-)
diff --git a/drivers/gpu/drm/amd/pm/powerplay/si_dpm.c
b/drivers/gpu/drm/amd/pm/powerplay/si_dpm.c
index 26a5321e621b..15c0b8af376f 100644
--- a/drivers/gpu/drm/amd/pm/powerplay/si_dpm.c
+++ b/drivers/gpu/drm/amd/pm/powerplay/si_dpm.c
@@ -4817,70 +4817,70 @@ static int si_populate_smc_initial_state(struct
amdgpu_device *adev,
u32 reg;
int ret;
- table->initialState.levels[0].mclk.vDLL_CNTL =
+ table->initialState.level.mclk.vDLL_CNTL =
cpu_to_be32(si_pi->clock_registers.dll_cntl);
- table->initialState.levels[0].mclk.vMCLK_PWRMGT_CNTL =
+ table->initialState.level.mclk.vMCLK_PWRMGT_CNTL =
cpu_to_be32(si_pi->clock_registers.mclk_pwrmgt_cntl);
- table->initialState.levels[0].mclk.vMPLL_AD_FUNC_CNTL =
+ table->initialState.level.mclk.vMPLL_AD_FUNC_CNTL =
cpu_to_be32(si_pi->clock_registers.mpll_ad_func_cntl);
- table->initialState.levels[0].mclk.vMPLL_DQ_FUNC_CNTL =
+ table->initialState.level.mclk.vMPLL_DQ_FUNC_CNTL =
cpu_to_be32(si_pi->clock_registers.mpll_dq_func_cntl);
- table->initialState.levels[0].mclk.vMPLL_FUNC_CNTL =
+ table->initialState.level.mclk.vMPLL_FUNC_CNTL =
cpu_to_be32(si_pi->clock_registers.mpll_func_cntl);
- table->initialState.levels[0].mclk.vMPLL_FUNC_CNTL_1 =
+ table->initialState.level.mclk.vMPLL_FUNC_CNTL_1 =
cpu_to_be32(si_pi->clock_registers.mpll_func_cntl_1);
- table->initialState.levels[0].mclk.vMPLL_FUNC_CNTL_2 =
+ table->initialState.level.mclk.vMPLL_FUNC_CNTL_2 =
cpu_to_be32(si_pi->clock_registers.mpll_func_cntl_2);
- table->initialState.levels[0].mclk.vMPLL_SS =
+ table->initialState.level.mclk.vMPLL_SS =
cpu_to_be32(si_pi->clock_registers.mpll_ss1);
- table->initialState.levels[0].mclk.vMPLL_SS2 =
+ table->initialState.level.mclk.vMPLL_SS2 =
cpu_to_be32(si_pi->clock_registers.mpll_ss2);
- table->initialState.levels[0].mclk.mclk_value =
+ table->initialState.level.mclk.mclk_value =
cpu_to_be32(initial_state->performance_levels[0].mclk);
- table->initialState.levels[0].sclk.vCG_SPLL_FUNC_CNTL =
+ table->initialState.level.sclk.vCG_SPLL_FUNC_CNTL =
cpu_to_be32(si_pi->clock_registers.cg_spll_func_cntl);
- table->initialState.levels[0].sclk.vCG_SPLL_FUNC_CNTL_2 =
+ table->initialState.level.sclk.vCG_SPLL_FUNC_CNTL_2 =
cpu_to_be32(si_pi->clock_registers.cg_spll_func_cntl_2);
- table->initialState.levels[0].sclk.vCG_SPLL_FUNC_CNTL_3 =
+ table->initialState.level.sclk.vCG_SPLL_FUNC_CNTL_3 =
cpu_to_be32(si_pi->clock_registers.cg_spll_func_cntl_3);
- table->initialState.levels[0].sclk.vCG_SPLL_FUNC_CNTL_4 =
+ table->initialState.level.sclk.vCG_SPLL_FUNC_CNTL_4 =
cpu_to_be32(si_pi->clock_registers.cg_spll_func_cntl_4);
- table-
Re: Radeon NI: GIT kernel with the nislands_smc commit doesn't boot on a Freescale P5040 board and P.A.Semi Nemo board
Hi Christian, On 5/8/21 06:33, Christian Zigotzky wrote: > Hi Gustavo, > > Your patch works! Thanks a lot! I tested it with my Freescale P5040 board and > P.A.Semi Nemo board with a connected AMD Radeon HD6970 NI graphics cards > (Cayman > XT) today. Awesome! :) Thank you! -- Gustavo > > Have a nice day, > Christian > > > On 07 May 2021 at 08:43am, Christian Zigotzky wrote: >> Hi Gustavo, >> >> Great! I will test it. Many thanks for your help. >> >> Cheers, >> Christian >> >> >>> On 7. May 2021, at 01:55, Gustavo A. R. Silva >>> wrote: >>> >>> Hi Christian, >>> >>>> On 4/30/21 06:59, Christian Zigotzky wrote: >>>> Hello, >>>> >>>> The Nemo board (A-EON AmigaOne X1000) [1] and the FSL P5040 Cyrus+ board >>>> (A-EON AmigaOne X5000) [2] with installed AMD Radeon HD6970 NI graphics >>>> cards (Cayman >>>> XT) [3] don't boot with the latest git kernel anymore after the commit >>>> "drm/radeon/nislands_smc.h: Replace one-element array with flexible-array >>>> member in >>>> struct NISLANDS_SMC_SWSTATE branch" [4]. This git kernel boots in a >>>> virtual e5500 QEMU machine with a VirtIO-GPU [5]. >>>> >>>> I bisected today [6]. >>>> >>>> Result: drm/radeon/nislands_smc.h: Replace one-element array with >>>> flexible-array member in struct NISLANDS_SMC_SWSTATE branch >>>> (434fb1e7444a2efc3a4ebd950c7f771ebfcffa31) [4] is the first bad commit. >>> I have a fix ready for this bug: >>> https://git.kernel.org/pub/scm/linux/kernel/git/gustavoars/linux.git/commit/?h=testing/drm-nislands >>> >>> I wonder if you could help me to test it with your environment, please. >>> It should be applied on top of mainline. >>> >>> Thank you! >>> -- >>> Gustavo >>> >>>> I was able to revert this commit [7] and after a new compiling, the kernel >>>> boots without any problems on my AmigaOnes. >>>> >>>> After that I created a patch for reverting this commit for new git test >>>> kernels. [3] >>>> >>>> The kernel compiles and boots with this patch on my AmigaOnes. Please find >>>> attached the kernel config files. >>>> >>>> Please check the first bad commit. >>>> >>>> Thanks, >>>> Christian >>>> >>>> [1] https://en.wikipedia.org/wiki/AmigaOne_X1000 >>>> [2] http://wiki.amiga.org/index.php?title=X5000 >>>> [3] https://forum.hyperion-entertainment.com/viewtopic.php?f=35=4377 >>>> [4] >>>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=434fb1e7444a2efc3a4ebd950c7f771ebfcffa31 >>>> [5] qemu-system-ppc64 -M ppce500 -cpu e5500 -m 1024 -kernel uImage -drive >>>> format=raw,file=MintPPC32-X5000.img,index=0,if=virtio -netdev >>>> user,id=mynet0 -device >>>> virtio-net-pci,netdev=mynet0 -append "rw root=/dev/vda" -device virtio-vga >>>> -usb -device usb-ehci,id=ehci -device usb-tablet -device >>>> virtio-keyboard-pci -smp 4 >>>> -vnc :1 >>>> [6] https://forum.hyperion-entertainment.com/viewtopic.php?p=53074#p53074 >>>> [7] git revert 434fb1e7444a2efc3a4ebd950c7f771ebfcffa3 >
[PATCH] drm/radeon/si_dpm: Fix SMU power state load
Create new structure SISLANDS_SMC_SWSTATE_SINGLE, as initialState.levels
and ACPIState.levels are never actually used as flexible arrays. Those
arrays can be used as simple objects of type
SISLANDS_SMC_HW_PERFORMANCE_LEVEL, instead.
Currently, the code fails because flexible array _levels_ in
struct SISLANDS_SMC_SWSTATE doesn't allow for code that access
the first element of initialState.levels and ACPIState.levels
arrays:
4353 table->initialState.levels[0].mclk.vDLL_CNTL =
4354 cpu_to_be32(si_pi->clock_registers.dll_cntl);
...
4555 table->ACPIState.levels[0].mclk.vDLL_CNTL =
4556 cpu_to_be32(dll_cntl);
because such element cannot exist without previously allocating
any dynamic memory for it (which never actually happens).
That's why struct SISLANDS_SMC_SWSTATE should only be used as type
for object driverState and new struct SISLANDS_SMC_SWSTATE_SINGLE is
created as type for objects initialState, ACPIState and ULVState.
Also, with the change from one-element array to flexible-array member
in commit 96e27e8d919e ("drm/radeon/si_dpm: Replace one-element array
with flexible-array in struct SISLANDS_SMC_SWSTATE"), the size of
dpmLevels in struct SISLANDS_SMC_STATETABLE should be fixed to be
SISLANDS_MAX_SMC_PERFORMANCE_LEVELS_PER_SWSTATE instead of
SISLANDS_MAX_SMC_PERFORMANCE_LEVELS_PER_SWSTATE - 1.
Bug: https://gitlab.freedesktop.org/drm/amd/-/issues/1583
Fixes: 96e27e8d919e ("drm/radeon/si_dpm: Replace one-element array with
flexible-array in struct SISLANDS_SMC_SWSTATE")
Cc: sta...@vger.kernel.org
Reported-by: Kai-Heng Feng
Signed-off-by: Gustavo A. R. Silva
---
drivers/gpu/drm/radeon/si_dpm.c | 174 +-
drivers/gpu/drm/radeon/sislands_smc.h | 34 +++--
2 files changed, 109 insertions(+), 99 deletions(-)
diff --git a/drivers/gpu/drm/radeon/si_dpm.c b/drivers/gpu/drm/radeon/si_dpm.c
index 91bfc4762767..2a8b9680cf6b 100644
--- a/drivers/gpu/drm/radeon/si_dpm.c
+++ b/drivers/gpu/drm/radeon/si_dpm.c
@@ -4350,70 +4350,70 @@ static int si_populate_smc_initial_state(struct
radeon_device *rdev,
u32 reg;
int ret;
- table->initialState.levels[0].mclk.vDLL_CNTL =
+ table->initialState.level.mclk.vDLL_CNTL =
cpu_to_be32(si_pi->clock_registers.dll_cntl);
- table->initialState.levels[0].mclk.vMCLK_PWRMGT_CNTL =
+ table->initialState.level.mclk.vMCLK_PWRMGT_CNTL =
cpu_to_be32(si_pi->clock_registers.mclk_pwrmgt_cntl);
- table->initialState.levels[0].mclk.vMPLL_AD_FUNC_CNTL =
+ table->initialState.level.mclk.vMPLL_AD_FUNC_CNTL =
cpu_to_be32(si_pi->clock_registers.mpll_ad_func_cntl);
- table->initialState.levels[0].mclk.vMPLL_DQ_FUNC_CNTL =
+ table->initialState.level.mclk.vMPLL_DQ_FUNC_CNTL =
cpu_to_be32(si_pi->clock_registers.mpll_dq_func_cntl);
- table->initialState.levels[0].mclk.vMPLL_FUNC_CNTL =
+ table->initialState.level.mclk.vMPLL_FUNC_CNTL =
cpu_to_be32(si_pi->clock_registers.mpll_func_cntl);
- table->initialState.levels[0].mclk.vMPLL_FUNC_CNTL_1 =
+ table->initialState.level.mclk.vMPLL_FUNC_CNTL_1 =
cpu_to_be32(si_pi->clock_registers.mpll_func_cntl_1);
- table->initialState.levels[0].mclk.vMPLL_FUNC_CNTL_2 =
+ table->initialState.level.mclk.vMPLL_FUNC_CNTL_2 =
cpu_to_be32(si_pi->clock_registers.mpll_func_cntl_2);
- table->initialState.levels[0].mclk.vMPLL_SS =
+ table->initialState.level.mclk.vMPLL_SS =
cpu_to_be32(si_pi->clock_registers.mpll_ss1);
- table->initialState.levels[0].mclk.vMPLL_SS2 =
+ table->initialState.level.mclk.vMPLL_SS2 =
cpu_to_be32(si_pi->clock_registers.mpll_ss2);
- table->initialState.levels[0].mclk.mclk_value =
+ table->initialState.level.mclk.mclk_value =
cpu_to_be32(initial_state->performance_levels[0].mclk);
- table->initialState.levels[0].sclk.vCG_SPLL_FUNC_CNTL =
+ table->initialState.level.sclk.vCG_SPLL_FUNC_CNTL =
cpu_to_be32(si_pi->clock_registers.cg_spll_func_cntl);
- table->initialState.levels[0].sclk.vCG_SPLL_FUNC_CNTL_2 =
+ table->initialState.level.sclk.vCG_SPLL_FUNC_CNTL_2 =
cpu_to_be32(si_pi->clock_registers.cg_spll_func_cntl_2);
- table->initialState.levels[0].sclk.vCG_SPLL_FUNC_CNTL_3 =
+ table->initialState.level.sclk.vCG_SPLL_FUNC_CNTL_3 =
cpu_to_be32(si_pi->clock_registers.cg_spll_func_cntl_3);
- table->initialState.levels[0].sclk.vCG_SPLL_FUNC_CNTL_4 =
+ table->initialState.level.sclk.vCG_SPLL_FUNC_CNTL_4 =
cpu_to_be32(si_pi->clock_registers.cg_spll_func_cntl_4);
- table->initialState.levels[0].sclk.vCG_SPLL_
[PATCH] drm/radeon/ni_dpm: Fix booting bug
Create new structure NISLANDS_SMC_SWSTATE_SINGLE, as initialState.levels
and ACPIState.levels are never actually used as flexible arrays. Those
arrays can be used as simple objects of type
NISLANDS_SMC_HW_PERFORMANCE_LEVEL, instead.
Currently, the code fails because flexible array _levels_ in
struct NISLANDS_SMC_SWSTATE doesn't allow for code that access
the first element of initialState.levels and ACPIState.levels
arrays:
drivers/gpu/drm/radeon/ni_dpm.c:
1690 table->initialState.levels[0].mclk.vMPLL_AD_FUNC_CNTL =
1691 cpu_to_be32(ni_pi->clock_registers.mpll_ad_func_cntl);
...
1903: table->ACPIState.levels[0].mclk.vMPLL_AD_FUNC_CNTL =
cpu_to_be32(mpll_ad_func_cntl);
1904: table->ACPIState.levels[0].mclk.vMPLL_AD_FUNC_CNTL_2 =
cpu_to_be32(mpll_ad_func_cntl_2);
because such element cannot exist without previously allocating
any dynamic memory for it (which never actually happens).
That's why struct NISLANDS_SMC_SWSTATE should only be used as type
for object driverState and new struct SISLANDS_SMC_SWSTATE_SINGLE is
created as type for objects initialState, ACPIState and ULVState.
Also, with the change from one-element array to flexible-array member
in commit 434fb1e7444a ("drm/radeon/nislands_smc.h: Replace one-element
array with flexible-array member in struct NISLANDS_SMC_SWSTATE"), the
size of dpmLevels in struct NISLANDS_SMC_STATETABLE should be fixed to
be NISLANDS_MAX_SMC_PERFORMANCE_LEVELS_PER_SWSTATE instead of
NISLANDS_MAX_SMC_PERFORMANCE_LEVELS_PER_SWSTATE - 1.
Bug:
https://lore.kernel.org/dri-devel/3eedbe78-1fbd-4763-a7f3-ac5665e76...@xenosoft.de/
Fixes: 434fb1e7444a ("drm/radeon/nislands_smc.h: Replace one-element array with
flexible-array member in struct NISLANDS_SMC_SWSTATE")
Cc: sta...@vger.kernel.org
Reported-by: Christian Zigotzky
Tested-by: Christian Zigotzky
Link:
https://lore.kernel.org/dri-devel/9bb5fcbd-daf5-1669-b3e7-b8624b3c3...@xenosoft.de/
Signed-off-by: Gustavo A. R. Silva
---
drivers/gpu/drm/radeon/ni_dpm.c | 144 +-
drivers/gpu/drm/radeon/nislands_smc.h | 34 +++---
2 files changed, 94 insertions(+), 84 deletions(-)
diff --git a/drivers/gpu/drm/radeon/ni_dpm.c b/drivers/gpu/drm/radeon/ni_dpm.c
index dd5ef6493723..769f666335ac 100644
--- a/drivers/gpu/drm/radeon/ni_dpm.c
+++ b/drivers/gpu/drm/radeon/ni_dpm.c
@@ -1687,102 +1687,102 @@ static int ni_populate_smc_initial_state(struct
radeon_device *rdev,
u32 reg;
int ret;
- table->initialState.levels[0].mclk.vMPLL_AD_FUNC_CNTL =
+ table->initialState.level.mclk.vMPLL_AD_FUNC_CNTL =
cpu_to_be32(ni_pi->clock_registers.mpll_ad_func_cntl);
- table->initialState.levels[0].mclk.vMPLL_AD_FUNC_CNTL_2 =
+ table->initialState.level.mclk.vMPLL_AD_FUNC_CNTL_2 =
cpu_to_be32(ni_pi->clock_registers.mpll_ad_func_cntl_2);
- table->initialState.levels[0].mclk.vMPLL_DQ_FUNC_CNTL =
+ table->initialState.level.mclk.vMPLL_DQ_FUNC_CNTL =
cpu_to_be32(ni_pi->clock_registers.mpll_dq_func_cntl);
- table->initialState.levels[0].mclk.vMPLL_DQ_FUNC_CNTL_2 =
+ table->initialState.level.mclk.vMPLL_DQ_FUNC_CNTL_2 =
cpu_to_be32(ni_pi->clock_registers.mpll_dq_func_cntl_2);
- table->initialState.levels[0].mclk.vMCLK_PWRMGT_CNTL =
+ table->initialState.level.mclk.vMCLK_PWRMGT_CNTL =
cpu_to_be32(ni_pi->clock_registers.mclk_pwrmgt_cntl);
- table->initialState.levels[0].mclk.vDLL_CNTL =
+ table->initialState.level.mclk.vDLL_CNTL =
cpu_to_be32(ni_pi->clock_registers.dll_cntl);
- table->initialState.levels[0].mclk.vMPLL_SS =
+ table->initialState.level.mclk.vMPLL_SS =
cpu_to_be32(ni_pi->clock_registers.mpll_ss1);
- table->initialState.levels[0].mclk.vMPLL_SS2 =
+ table->initialState.level.mclk.vMPLL_SS2 =
cpu_to_be32(ni_pi->clock_registers.mpll_ss2);
- table->initialState.levels[0].mclk.mclk_value =
+ table->initialState.level.mclk.mclk_value =
cpu_to_be32(initial_state->performance_levels[0].mclk);
- table->initialState.levels[0].sclk.vCG_SPLL_FUNC_CNTL =
+ table->initialState.level.sclk.vCG_SPLL_FUNC_CNTL =
cpu_to_be32(ni_pi->clock_registers.cg_spll_func_cntl);
- table->initialState.levels[0].sclk.vCG_SPLL_FUNC_CNTL_2 =
+ table->initialState.level.sclk.vCG_SPLL_FUNC_CNTL_2 =
cpu_to_be32(ni_pi->clock_registers.cg_spll_func_cntl_2);
- table->initialState.levels[0].sclk.vCG_SPLL_FUNC_CNTL_3 =
+ table->initialState.level.sclk.vCG_SPLL_FUNC_CNTL_3 =
cpu_to_be32(ni_pi->clock_registers.cg_spll_func_cntl_3);
- table->initialState.levels[0].sclk.vCG_SPLL_FUNC_CNTL_4 =
+ table->initialSta
Re: Radeon NI: GIT kernel with the nislands_smc commit doesn't boot on a Freescale P5040 board and P.A.Semi Nemo board
Hi Christian, On 4/30/21 06:59, Christian Zigotzky wrote: > Hello, > > The Nemo board (A-EON AmigaOne X1000) [1] and the FSL P5040 Cyrus+ board > (A-EON AmigaOne X5000) [2] with installed AMD Radeon HD6970 NI graphics cards > (Cayman > XT) [3] don't boot with the latest git kernel anymore after the commit > "drm/radeon/nislands_smc.h: Replace one-element array with flexible-array > member in > struct NISLANDS_SMC_SWSTATE branch" [4]. This git kernel boots in a virtual > e5500 QEMU machine with a VirtIO-GPU [5]. > > I bisected today [6]. > > Result: drm/radeon/nislands_smc.h: Replace one-element array with > flexible-array member in struct NISLANDS_SMC_SWSTATE branch > (434fb1e7444a2efc3a4ebd950c7f771ebfcffa31) [4] is the first bad commit. I have a fix ready for this bug: https://git.kernel.org/pub/scm/linux/kernel/git/gustavoars/linux.git/commit/?h=testing/drm-nislands I wonder if you could help me to test it with your environment, please. It should be applied on top of mainline. Thank you! -- Gustavo > > I was able to revert this commit [7] and after a new compiling, the kernel > boots without any problems on my AmigaOnes. > > After that I created a patch for reverting this commit for new git test > kernels. [3] > > The kernel compiles and boots with this patch on my AmigaOnes. Please find > attached the kernel config files. > > Please check the first bad commit. > > Thanks, > Christian > > [1] https://en.wikipedia.org/wiki/AmigaOne_X1000 > [2] http://wiki.amiga.org/index.php?title=X5000 > [3] https://forum.hyperion-entertainment.com/viewtopic.php?f=35=4377 > [4] > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=434fb1e7444a2efc3a4ebd950c7f771ebfcffa31 > [5] qemu-system-ppc64 -M ppce500 -cpu e5500 -m 1024 -kernel uImage -drive > format=raw,file=MintPPC32-X5000.img,index=0,if=virtio -netdev user,id=mynet0 > -device > virtio-net-pci,netdev=mynet0 -append "rw root=/dev/vda" -device virtio-vga > -usb -device usb-ehci,id=ehci -device usb-tablet -device virtio-keyboard-pci > -smp 4 > -vnc :1 > [6] https://forum.hyperion-entertainment.com/viewtopic.php?p=53074#p53074 > [7] git revert 434fb1e7444a2efc3a4ebd950c7f771ebfcffa3
Re: Radeon NI: GIT kernel with the nislands_smc commit doesn't boot on a Freescale P5040 board and P.A.Semi Nemo board
On 4/30/21 10:26, Deucher, Alexander wrote: > [AMD Public Use] > > + Gustavo, amd-gfx > >> -Original Message- >> From: Christian Zigotzky >> Sent: Friday, April 30, 2021 8:00 AM >> To: gustavo...@kernel.org; Deucher, Alexander >> >> Cc: R.T.Dickinson ; Darren Stevens > zone.net>; mad skateman ; linuxppc-dev >> ; Olof Johansson ; >> Maling list - DRI developers ; Michel >> Dänzer ; Christian Zigotzky >> Subject: Radeon NI: GIT kernel with the nislands_smc commit doesn't >> boot on a Freescale P5040 board and P.A.Semi Nemo board >> >> Hello, >> >> The Nemo board (A-EON AmigaOne X1000) [1] and the FSL P5040 Cyrus+ >> board (A-EON AmigaOne X5000) [2] with installed AMD Radeon HD6970 NI >> graphics cards (Cayman XT) [3] don't boot with the latest git kernel >> anymore after the commit "drm/radeon/nislands_smc.h: Replace >> one-element array with flexible-array member in struct NISLANDS_SMC_SWSTATE >> branch" [4]. >> This git kernel boots in a virtual e5500 QEMU machine with a VirtIO-GPU [5]. >> >> I bisected today [6]. >> >> Result: drm/radeon/nislands_smc.h: Replace one-element array with >> flexible-array member in struct NISLANDS_SMC_SWSTATE branch >> (434fb1e7444a2efc3a4ebd950c7f771ebfcffa31) [4] is the first bad commit. >> >> I was able to revert this commit [7] and after a new compiling, the >> kernel boots without any problems on my AmigaOnes. >> >> After that I created a patch for reverting this commit for new git test >> kernels. >> [3] >> >> The kernel compiles and boots with this patch on my AmigaOnes. Please >> find attached the kernel config files. >> >> Please check the first bad commit. I'll have a look. Thanks for the report! -- Gustavo >> >> Thanks, >> Christian >> >> [1] >> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fen. >> wikipedia.org%2Fwiki%2FAmigaOne_X1000data=04%7C01%7Calexand >> er.deucher%40amd.com%7C0622549383fb4320346b08d90bcf7be1%7C3dd89 >> 61fe4884e608e11a82d994e183d%7C0%7C0%7C637553808670161651%7CUnkn >> own%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik >> 1haWwiLCJXVCI6Mn0%3D%7C1000sdata=PNSrApUdMrku20hH7dEKlJJ >> TBi7Qp5JOkqpA4MvKqdE%3Dreserved=0 >> [2] >> https://nam11.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwiki. >> a miga.org%2Findex.php%3Ftitle%3DX5000data=04%7C01%7Calexander >> .deucher%40amd.com%7C0622549383fb4320346b08d90bcf7be1%7C3dd8961f >> e4884e608e11a82d994e183d%7C0%7C0%7C637553808670161651%7CUnknow >> n%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1ha >> WwiLCJXVCI6Mn0%3D%7C1000sdata=B8Uvhs25%2FP3RfnL1AgICN3Y4 >> CEXeCE1yIoi3vvwvGto%3Dreserved=0 >> [3] >> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fforu >> m.hyperion- >> entertainment.com%2Fviewtopic.php%3Ff%3D35%26t%3D4377data= >> 04%7C01%7Calexander.deucher%40amd.com%7C0622549383fb4320346b08d >> 90bcf7be1%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C63755380 >> 8670161651%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIj >> oiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000sdata=TokXplD >> Tvg3%2BZMPLCgR1fs%2BN2X9MIfLXLW67MAM2Qsk%3Dreserved=0 >> [4] >> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit. >> k ernel.org%2Fpub%2Fscm%2Flinux%2Fkernel%2Fgit%2Ftorvalds%2Flinux.git% >> 2Fcommit%2F%3Fid%3D434fb1e7444a2efc3a4ebd950c7f771ebfcffa31d >> ata=04%7C01%7Calexander.deucher%40amd.com%7C0622549383fb4320346 >> b08d90bcf7be1%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C6375 >> 53808670161651%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiL >> CJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000sdata=JC >> M4xvPEnWdckcTPbQ2Ujv%2FAiMMsFMzzl4Pr%2FRPlcMQ%3Dreserve >> d=0 >> [5] qemu-system-ppc64 -M ppce500 -cpu e5500 -m 1024 -kernel uImage - >> drive format=raw,file=MintPPC32-X5000.img,index=0,if=virtio -netdev >> user,id=mynet0 -device virtio-net-pci,netdev=mynet0 -append "rw >> root=/dev/vda" -device virtio-vga -usb -device usb-ehci,id=ehci >> -device usb- tablet -device virtio-keyboard-pci -smp 4 -vnc :1 [6] >> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fforu >> m.hyperion- >> entertainment.com%2Fviewtopic.php%3Fp%3D53074%23p53074data >> =04%7C01%7Calexander.deucher%40amd.com%7C0622549383fb4320346b08 >> d90bcf7be1%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C6375538 >> 08670161651%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQ >> IjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000sdata=RXfSlY >> A3bDEFas0%2Fk2vMWsl2l0nuhS2ecjZgSBLc%2Bs4%3Dreserved=0 >> [7] git revert 434fb1e7444a2efc3a4ebd950c7f771ebfcffa3 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH RESEND][next] drm/nouveau: Fix fall-through warnings for Clang
Hi all, Friendly ping: who can take this, please? Thanks -- Gustavo On 3/5/21 03:56, Gustavo A. R. Silva wrote: > In preparation to enable -Wimplicit-fallthrough for Clang, fix a couple > of warnings by explicitly adding a couple of break statements instead > of letting the code fall through to the next case. > > Link: https://github.com/KSPP/linux/issues/115 > Signed-off-by: Gustavo A. R. Silva > --- > drivers/gpu/drm/nouveau/nouveau_bo.c| 1 + > drivers/gpu/drm/nouveau/nouveau_connector.c | 1 + > 2 files changed, 2 insertions(+) > > diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c > b/drivers/gpu/drm/nouveau/nouveau_bo.c > index 2375711877cf..62903c3b368d 100644 > --- a/drivers/gpu/drm/nouveau/nouveau_bo.c > +++ b/drivers/gpu/drm/nouveau/nouveau_bo.c > @@ -443,6 +443,7 @@ nouveau_bo_pin(struct nouveau_bo *nvbo, uint32_t domain, > bool contig) > break; > case TTM_PL_TT: > error |= !(domain & NOUVEAU_GEM_DOMAIN_GART); > + break; > default: > break; > } > diff --git a/drivers/gpu/drm/nouveau/nouveau_connector.c > b/drivers/gpu/drm/nouveau/nouveau_connector.c > index 61e6d7412505..eb844cdcaec2 100644 > --- a/drivers/gpu/drm/nouveau/nouveau_connector.c > +++ b/drivers/gpu/drm/nouveau/nouveau_connector.c > @@ -157,6 +157,7 @@ nouveau_conn_atomic_set_property(struct drm_connector > *connector, > default: > break; > } > + break; > case DRM_MODE_SCALE_FULLSCREEN: > case DRM_MODE_SCALE_CENTER: > case DRM_MODE_SCALE_ASPECT: > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH RESEND][next] drm/nouveau/therm: Fix fall-through warnings for Clang
Hi all, Friendly ping: who can take this, please? Thanks -- Gustavo On 3/5/21 03:58, Gustavo A. R. Silva wrote: > In preparation to enable -Wimplicit-fallthrough for Clang, fix a warning > by explicitly adding a break statement instead of letting the code fall > through to the next case. > > Link: https://github.com/KSPP/linux/issues/115 > Signed-off-by: Gustavo A. R. Silva > --- > drivers/gpu/drm/nouveau/nvkm/subdev/therm/gf119.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/therm/gf119.c > b/drivers/gpu/drm/nouveau/nvkm/subdev/therm/gf119.c > index 2b031d4eaeb6..684aff7437ee 100644 > --- a/drivers/gpu/drm/nouveau/nvkm/subdev/therm/gf119.c > +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/therm/gf119.c > @@ -41,6 +41,7 @@ pwm_info(struct nvkm_therm *therm, int line) > default: > break; > } > + break; > default: > break; > } > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH RESEND][next] drm/nouveau/clk: Fix fall-through warnings for Clang
Hi all, Friendly ping: who can take this, please? Thanks -- Gustavo On 3/5/21 03:56, Gustavo A. R. Silva wrote: > In preparation to enable -Wimplicit-fallthrough for Clang, fix a warning > by explicitly adding a break statement instead of letting the code fall > through to the next case. > > Link: https://github.com/KSPP/linux/issues/115 > Signed-off-by: Gustavo A. R. Silva > --- > drivers/gpu/drm/nouveau/nvkm/subdev/clk/nv50.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/clk/nv50.c > b/drivers/gpu/drm/nouveau/nvkm/subdev/clk/nv50.c > index 83067763c0ec..e1d31c62f9ec 100644 > --- a/drivers/gpu/drm/nouveau/nvkm/subdev/clk/nv50.c > +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/clk/nv50.c > @@ -313,6 +313,7 @@ nv50_clk_read(struct nvkm_clk *base, enum nv_clk_src src) > default: > break; > } > + break; > default: > break; > } > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH][next] drm/amd/display: Fix sizeof arguments in bw_calcs_init()
On 3/22/21 09:04, Chen, Guchun wrote: > [AMD Public Use] > > Thanks for your patch, Silva. The issue has been fixed by " a5c6007e20e1 > drm/amd/display: fix modprobe failure on vega series". Great. :) Good to know this is already fixed. Thanks! -- Gustavo ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH][next] drm/amd/display: Fix sizeof arguments in bw_calcs_init()
The wrong sizeof values are currently being used as arguments to
kzalloc().
Fix this by using the right arguments *dceip and *vbios,
correspondingly.
Addresses-Coverity-ID: 1502901 ("Wrong sizeof argument")
Fixes: fca1e079055e ("drm/amd/display/dc/calcs/dce_calcs: Remove some large
variables from the stack")
Signed-off-by: Gustavo A. R. Silva
---
drivers/gpu/drm/amd/display/dc/calcs/dce_calcs.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/dc/calcs/dce_calcs.c
b/drivers/gpu/drm/amd/display/dc/calcs/dce_calcs.c
index 556ecfabc8d2..1244fcb0f446 100644
--- a/drivers/gpu/drm/amd/display/dc/calcs/dce_calcs.c
+++ b/drivers/gpu/drm/amd/display/dc/calcs/dce_calcs.c
@@ -2051,11 +2051,11 @@ void bw_calcs_init(struct bw_calcs_dceip *bw_dceip,
enum bw_calcs_version version = bw_calcs_version_from_asic_id(asic_id);
- dceip = kzalloc(sizeof(dceip), GFP_KERNEL);
+ dceip = kzalloc(sizeof(*dceip), GFP_KERNEL);
if (!dceip)
return;
- vbios = kzalloc(sizeof(vbios), GFP_KERNEL);
+ vbios = kzalloc(sizeof(*vbios), GFP_KERNEL);
if (!vbios) {
kfree(dceip);
return;
--
2.27.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drivers/video/fbdev:modify 0 to NULL
On 3/17/21 21:47, Chunyou Tang wrote: > I think "if (info == NULL)" is more intuitive,and there have many > compare likes "if (info == NULL)" in this file. In that case, all those instances should be changed to if (!foo), instead. -- Gustavo ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drivers/video/fbdev:modify 0 to NULL
On 3/17/21 21:33, ChunyouTang wrote:
> From: tangchunyou
>
> modify 0 to NULL,info is a pointer,it should be
>
> compared with NULL,not 0
>
> Signed-off-by: tangchunyou
> ---
> drivers/video/fbdev/offb.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/video/fbdev/offb.c b/drivers/video/fbdev/offb.c
> index 4501e84..cd1042f 100644
> --- a/drivers/video/fbdev/offb.c
> +++ b/drivers/video/fbdev/offb.c
> @@ -412,7 +412,7 @@ static void __init offb_init_fb(const char *name,
>
> info = framebuffer_alloc(sizeof(u32) * 16, NULL);
>
> - if (info == 0) {
> + if (info == NULL) {
if (!info) is better.
--
Gustavo
> release_mem_region(res_start, res_size);
> return;
> }
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH][next] drm/radeon/si_dpm: Replace one-element array with flexible-array in struct SISLANDS_SMC_SWSTATE
On Fri, Mar 05, 2021 at 02:10:44PM -0500, Alex Deucher wrote: > Applied. Thanks! Awesome. :) Thanks, Alex. -- Gustavo ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH RESEND][next] drm/nouveau/therm: Fix fall-through warnings for Clang
In preparation to enable -Wimplicit-fallthrough for Clang, fix a warning by explicitly adding a break statement instead of letting the code fall through to the next case. Link: https://github.com/KSPP/linux/issues/115 Signed-off-by: Gustavo A. R. Silva --- drivers/gpu/drm/nouveau/nvkm/subdev/therm/gf119.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/therm/gf119.c b/drivers/gpu/drm/nouveau/nvkm/subdev/therm/gf119.c index 2b031d4eaeb6..684aff7437ee 100644 --- a/drivers/gpu/drm/nouveau/nvkm/subdev/therm/gf119.c +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/therm/gf119.c @@ -41,6 +41,7 @@ pwm_info(struct nvkm_therm *therm, int line) default: break; } + break; default: break; } -- 2.27.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH RESEND][next] drm/nouveau: Fix fall-through warnings for Clang
In preparation to enable -Wimplicit-fallthrough for Clang, fix a couple of warnings by explicitly adding a couple of break statements instead of letting the code fall through to the next case. Link: https://github.com/KSPP/linux/issues/115 Signed-off-by: Gustavo A. R. Silva --- drivers/gpu/drm/nouveau/nouveau_bo.c| 1 + drivers/gpu/drm/nouveau/nouveau_connector.c | 1 + 2 files changed, 2 insertions(+) diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c b/drivers/gpu/drm/nouveau/nouveau_bo.c index 2375711877cf..62903c3b368d 100644 --- a/drivers/gpu/drm/nouveau/nouveau_bo.c +++ b/drivers/gpu/drm/nouveau/nouveau_bo.c @@ -443,6 +443,7 @@ nouveau_bo_pin(struct nouveau_bo *nvbo, uint32_t domain, bool contig) break; case TTM_PL_TT: error |= !(domain & NOUVEAU_GEM_DOMAIN_GART); + break; default: break; } diff --git a/drivers/gpu/drm/nouveau/nouveau_connector.c b/drivers/gpu/drm/nouveau/nouveau_connector.c index 61e6d7412505..eb844cdcaec2 100644 --- a/drivers/gpu/drm/nouveau/nouveau_connector.c +++ b/drivers/gpu/drm/nouveau/nouveau_connector.c @@ -157,6 +157,7 @@ nouveau_conn_atomic_set_property(struct drm_connector *connector, default: break; } + break; case DRM_MODE_SCALE_FULLSCREEN: case DRM_MODE_SCALE_CENTER: case DRM_MODE_SCALE_ASPECT: -- 2.27.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
