Re: [PATCH v2] x86/tdx: replace deprecated strncpy with strtomem_pad
* Justin Stitt wrote:
> strncpy works perfectly here in all cases, however, it is deprecated and
> as such we should prefer more robust and less ambiguous string apis.
>
> Let's use `strtomem_pad` as this matches the functionality of `strncpy`
> and is _not_ deprecated.
>
> Link:
> https://www.kernel.org/doc/html/latest/process/deprecated.html#strncpy-on-nul-terminated-strings
> [1]
> Link: https://github.com/KSPP/linux/issues/90
> Cc: linux-hardening@vger.kernel.org
> Cc: Kees Cook
> Signed-off-by: Justin Stitt
> ---
> Changes in v2:
> - update subject (thanks Kees)
> - update commit message (thanks Dave)
> - rebase onto mainline cbf3a2cb156a2c91
> - Link to v1:
> https://lore.kernel.org/r/20230911-strncpy-arch-x86-coco-tdx-tdx-c-v1-1-4b3815572...@google.com
> ---
> Note: build-tested
>
> Note: Ingo Molnar has some concerns about the comment being out of sync
> [1] but I believe the comment still has a place as we can still
> theoretically copy 64 bytes into our destination buffer without a
> NUL-byte. The extra information about the 65th byte being NUL may serve
> helpful to future travelers of this code. What do we think? I can drop
> the comment in a v3 if needed.
> /* VMM assumes '\0' in byte 65, if the message took all 64 bytes */
> - strncpy(message.str, msg, 64);
> + strtomem_pad(message.str, msg, '\0');
My concern was that with the old code it was obvious that the size
of message.str was 64 bytes - but I judged this based on the
patch context alone, which seemingly lost context due to the change.
In reality it's easy to see it when reading the code, because the
length definition is right before the code:
union {
/* Define register order according to the GHCI */
struct { u64 r14, r15, rbx, rdi, rsi, r8, r9, rdx; };
char str[64];
^
} message;
/* VMM assumes '\0' in byte 65, if the message took all 64 bytes */
strtomem_pad(message.str, msg, '\0');
:-)
So no worries - I've applied your patch to tip:x86/mm as-is.
Thanks,
Ingo
Re: [PATCH v1 1/1] kernel.h: Move lib/cmdline.c prototypes to string.h
On Wed, Oct 4, 2023 at 2:39 AM Kees Cook wrote: > > On Tue, Oct 03, 2023 at 04:01:42PM +0300, Andy Shevchenko wrote: > > The lib/cmdline.c is basically a set of some small string parsers > > which are wide used in the kernel. Their prototypes belong to the > > string.h rather then kernel.h. > > > > Signed-off-by: Andy Shevchenko > > I think these should live in string_helpers.h not string.h (which is, in > theory, supposed to be used for the standard C string library functions, > though that's not 100% currently)... These are being used in the early stages where usually we have string.h. So, I would argue, but if you insist, I can move them. What about lib/argv_slit.c then? Because semantically it's quite close to what the lib/cmdline.c is doing. -- With Best Regards, Andy Shevchenko
[PATCH v2] net: sched: cls_u32: Fix allocation size in u32_init()
commit d61491a51f7e ("net/sched: cls_u32: Replace one-element array
with flexible-array member") incorrecly replaced an instance of
`sizeof(*tp_c)` with `struct_size(tp_c, hlist->ht, 1)`. This results
in a an over-allocation of 8 bytes.
This change is wrong because `hlist` in `struct tc_u_common` is a
pointer:
net/sched/cls_u32.c:
struct tc_u_common {
struct tc_u_hnode __rcu *hlist;
void*ptr;
int refcnt;
struct idr handle_idr;
struct hlist_node hnode;
longknodes;
};
So, the use of `struct_size()` makes no sense: we don't need to allocate
any extra space for a flexible-array member. `sizeof(*tp_c)` is just fine.
So, `struct_size(tp_c, hlist->ht, 1)` translates to:
sizeof(*tp_c) + sizeof(tp_c->hlist->ht) ==
sizeof(struct tc_u_common) + sizeof(struct tc_u_knode *) ==
144 + 8 == 0x98 (byes)
^^^
|
unnecessary extra
allocation size
$ pahole -C tc_u_common net/sched/cls_u32.o
struct tc_u_common {
struct tc_u_hnode *hlist;/* 0 8 */
void * ptr; /* 8 8 */
intrefcnt; /*16 4 */
/* XXX 4 bytes hole, try to pack */
struct idr handle_idr; /*2496 */
/* --- cacheline 1 boundary (64 bytes) was 56 bytes ago --- */
struct hlist_node hnode;/* 12016 */
/* --- cacheline 2 boundary (128 bytes) was 8 bytes ago --- */
long int knodes; /* 136 8 */
/* size: 144, cachelines: 3, members: 6 */
/* sum members: 140, holes: 1, sum holes: 4 */
/* last cacheline: 16 bytes */
};
And with `sizeof(*tp_c)`, we have:
sizeof(*tp_c) == sizeof(struct tc_u_common) == 144 == 0x90 (bytes)
which is the correct and original allocation size.
Fix this issue by replacing `struct_size(tp_c, hlist->ht, 1)` with
`sizeof(*tp_c)`, and avoid allocating 8 too many bytes.
The following difference in binary output is expected and reflects the
desired change:
| net/sched/cls_u32.o
| @@ -6148,7 +6148,7 @@
| include/linux/slab.h:599
| 2cf5: mov0x0(%rip),%rdi# 2cfc
|2cf8: R_X86_64_PC32 kmalloc_caches+0xc
|-2cfc: mov$0x98,%edx
|+2cfc: mov$0x90,%edx
Reported-by: Alejandro Colomar
Closes:
https://lore.kernel.org/lkml/09b4a2ce-da74-3a19-6961-67883f634...@kernel.org/
Signed-off-by: Gustavo A. R. Silva
---
Changes in v2:
- Update subject line.
- Update changelog text.
v1:
- Link: https://lore.kernel.org/linux-hardening/ZN5DvRyq6JNz20l1@work/
net/sched/cls_u32.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/sched/cls_u32.c b/net/sched/cls_u32.c
index da4c179a4d41..6663e971a13e 100644
--- a/net/sched/cls_u32.c
+++ b/net/sched/cls_u32.c
@@ -366,7 +366,7 @@ static int u32_init(struct tcf_proto *tp)
idr_init(&root_ht->handle_idr);
if (tp_c == NULL) {
- tp_c = kzalloc(struct_size(tp_c, hlist->ht, 1), GFP_KERNEL);
+ tp_c = kzalloc(sizeof(*tp_c), GFP_KERNEL);
if (tp_c == NULL) {
kfree(root_ht);
return -ENOBUFS;
--
2.34.1
Re: [PATCH] dmaengine: fsl-edma: Annotate struct struct fsl_edma_engine with __counted_by
On Tue, 03 Oct 2023 16:27:56 -0700, 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 struct > fsl_edma_engine. > > [...] Applied, thanks! [1/1] dmaengine: fsl-edma: Annotate struct struct fsl_edma_engine with __counted_by commit: c223bafdcbd51506df00509088efc62e08ef6c3f Best regards, -- ~Vinod
Re: [PATCH] dmaengine: ep93xx_dma: Annotate struct ep93xx_dma_engine with __counted_by
On Thu, 28 Sep 2023 16:43:42 -0700, 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 ep93xx_dma_engine. > > [...] Applied, thanks! [1/1] dmaengine: ep93xx_dma: Annotate struct ep93xx_dma_engine with __counted_by commit: 74d885711c2f619e92f41ef308691948cc63f224 Best regards, -- ~Vinod
Re: [PATCH v2] net: sched: cls_u32: Fix allocation size in u32_init()
On Wed, Oct 4, 2023 at 9:19 AM Gustavo A. R. Silva
wrote:
>
> commit d61491a51f7e ("net/sched: cls_u32: Replace one-element array
> with flexible-array member") incorrecly replaced an instance of
> `sizeof(*tp_c)` with `struct_size(tp_c, hlist->ht, 1)`. This results
> in a an over-allocation of 8 bytes.
>
> This change is wrong because `hlist` in `struct tc_u_common` is a
> pointer:
>
> net/sched/cls_u32.c:
> struct tc_u_common {
> struct tc_u_hnode __rcu *hlist;
> void*ptr;
> int refcnt;
> struct idr handle_idr;
> struct hlist_node hnode;
> longknodes;
> };
>
> So, the use of `struct_size()` makes no sense: we don't need to allocate
> any extra space for a flexible-array member. `sizeof(*tp_c)` is just fine.
>
> So, `struct_size(tp_c, hlist->ht, 1)` translates to:
>
> sizeof(*tp_c) + sizeof(tp_c->hlist->ht) ==
> sizeof(struct tc_u_common) + sizeof(struct tc_u_knode *) ==
> 144 + 8 == 0x98 (byes)
> ^^^
> |
> unnecessary extra
> allocation size
>
> $ pahole -C tc_u_common net/sched/cls_u32.o
> struct tc_u_common {
> struct tc_u_hnode *hlist;/* 0 8 */
> void * ptr; /* 8 8 */
> intrefcnt; /*16 4 */
>
> /* XXX 4 bytes hole, try to pack */
>
> struct idr handle_idr; /*2496 */
> /* --- cacheline 1 boundary (64 bytes) was 56 bytes ago --- */
> struct hlist_node hnode;/* 12016 */
> /* --- cacheline 2 boundary (128 bytes) was 8 bytes ago --- */
> long int knodes; /* 136 8 */
>
> /* size: 144, cachelines: 3, members: 6 */
> /* sum members: 140, holes: 1, sum holes: 4 */
> /* last cacheline: 16 bytes */
> };
>
> And with `sizeof(*tp_c)`, we have:
>
> sizeof(*tp_c) == sizeof(struct tc_u_common) == 144 == 0x90 (bytes)
>
> which is the correct and original allocation size.
>
> Fix this issue by replacing `struct_size(tp_c, hlist->ht, 1)` with
> `sizeof(*tp_c)`, and avoid allocating 8 too many bytes.
>
> The following difference in binary output is expected and reflects the
> desired change:
>
> | net/sched/cls_u32.o
> | @@ -6148,7 +6148,7 @@
> | include/linux/slab.h:599
> | 2cf5: mov0x0(%rip),%rdi# 2cfc
> |2cf8: R_X86_64_PC32 kmalloc_caches+0xc
> |-2cfc: mov$0x98,%edx
> |+2cfc: mov$0x90,%edx
>
> Reported-by: Alejandro Colomar
> Closes:
> https://lore.kernel.org/lkml/09b4a2ce-da74-3a19-6961-67883f634...@kernel.org/
> Signed-off-by: Gustavo A. R. Silva
Acked-by: Jamal Hadi Salim
cheers,
jamal
> Changes in v2:
> - Update subject line.
> - Update changelog text.
>
> v1:
> - Link: https://lore.kernel.org/linux-hardening/ZN5DvRyq6JNz20l1@work/
>
> net/sched/cls_u32.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/sched/cls_u32.c b/net/sched/cls_u32.c
> index da4c179a4d41..6663e971a13e 100644
> --- a/net/sched/cls_u32.c
> +++ b/net/sched/cls_u32.c
> @@ -366,7 +366,7 @@ static int u32_init(struct tcf_proto *tp)
> idr_init(&root_ht->handle_idr);
>
> if (tp_c == NULL) {
> - tp_c = kzalloc(struct_size(tp_c, hlist->ht, 1), GFP_KERNEL);
> + tp_c = kzalloc(sizeof(*tp_c), GFP_KERNEL);
> if (tp_c == NULL) {
> kfree(root_ht);
> return -ENOBUFS;
> --
> 2.34.1
>
Re: [PATCH] ASoC: soc-dapm: Annotate struct snd_soc_dapm_widget_list with __counted_by
On Tue, 03 Oct 2023 16:28:53 -0700, 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 > snd_soc_dapm_widget_list. > Additionally, since the element count member must be set before accessing > the annotated flexible array member, move its initialization earlier. > > [...] Applied to https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next Thanks! [1/1] ASoC: soc-dapm: Annotate struct snd_soc_dapm_widget_list with __counted_by commit: 80e698e2df5ba2124bdeca37f1e589de58a4d514 All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted. You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed. If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced. Please add any relevant lists and maintainers to the CCs when replying to this mail. Thanks, Mark
[linux-next:master] BUILD REGRESSION 33b64befb1a28bca3f5a9ed9807d2f87e976c63a
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git master branch HEAD: 33b64befb1a28bca3f5a9ed9807d2f87e976c63a Add linux-next specific files for 20231004 Error/Warning reports: https://lore.kernel.org/oe-kbuild-all/202309122047.cri9yjrq-...@intel.com https://lore.kernel.org/oe-kbuild-all/202309192314.vbsjiim5-...@intel.com https://lore.kernel.org/oe-kbuild-all/202309212121.cul1ptra-...@intel.com https://lore.kernel.org/oe-kbuild-all/202309212339.hxhbu2f1-...@intel.com https://lore.kernel.org/oe-kbuild-all/202309221945.uwcq56zg-...@intel.com https://lore.kernel.org/oe-kbuild-all/202310041744.d34giv9v-...@intel.com https://lore.kernel.org/oe-kbuild-all/202310042215.w9pg3rqs-...@intel.com https://lore.kernel.org/oe-kbuild-all/202310042342.bv7kezcd-...@intel.com Error/Warning: (recently discovered and may have been fixed) Documentation/gpu/amdgpu/thermal:43: ./drivers/gpu/drm/amd/pm/amdgpu_pm.c:988: WARNING: Unexpected indentation. drivers/cpufreq/sti-cpufreq.c:215:50: warning: '%d' directive output may be truncated writing between 1 and 10 bytes into a region of size 2 [-Wformat-truncation=] drivers/net/ethernet/intel/ice/ice_dpll.c:1066: undefined reference to `ice_get_cgu_state' drivers/net/ethernet/intel/ice/ice_dpll.c:1666: undefined reference to `ice_cgu_get_pin_freq_supp' drivers/net/ethernet/intel/ice/ice_dpll.c:1802: undefined reference to `ice_get_cgu_rclk_pin_info' drivers/net/ethernet/intel/ice/ice_lib.c:3979: undefined reference to `ice_is_phy_rclk_present' fs/bcachefs/bcachefs_format.h:215:25: warning: 'p' offset 3 in 'struct bkey' isn't aligned to 4 [-Wpacked-not-aligned] fs/bcachefs/bcachefs_format.h:217:25: warning: 'version' offset 27 in 'struct bkey' isn't aligned to 4 [-Wpacked-not-aligned] fs/gfs2/inode.c:1876:14: sparse:struct gfs2_glock * fs/gfs2/inode.c:1876:14: sparse:struct gfs2_glock [noderef] __rcu * fs/gfs2/super.c:1543:17: sparse:struct gfs2_glock * fs/gfs2/super.c:1543:17: sparse:struct gfs2_glock [noderef] __rcu * include/linux/fortify-string.h:57:33: warning: writing 8 bytes into a region of size 0 [-Wstringop-overflow=] kernel/bpf/helpers.c:1906:19: warning: no previous declaration for 'bpf_percpu_obj_new_impl' [-Wmissing-declarations] kernel/bpf/helpers.c:1942:18: warning: no previous declaration for 'bpf_percpu_obj_drop_impl' [-Wmissing-declarations] kernel/bpf/helpers.c:2477:18: warning: no previous declaration for 'bpf_throw' [-Wmissing-declarations] ld: drivers/net/ethernet/intel/ice/ice_dpll.c:1646: undefined reference to `ice_cgu_get_pin_name' ld: drivers/net/ethernet/intel/ice/ice_dpll.c:1647: undefined reference to `ice_cgu_get_pin_type' ld: drivers/net/ethernet/intel/ice/ice_lib.c:3984: undefined reference to `ice_is_cgu_present' ld: drivers/net/ethernet/intel/ice/ice_lib.c:3986: undefined reference to `ice_is_clock_mux_present_e810t' loongarch64-linux-ld: ice_dpll.c:(.text+0xb78): undefined reference to `ice_cgu_get_pin_type' s390-linux-ld: ice_dpll.c:(.text+0xa4c): undefined reference to `ice_cgu_get_pin_type' s390-linux-ld: ice_dpll.c:(.text+0xaf0): undefined reference to `ice_cgu_get_pin_freq_supp' s390-linux-ld: ice_lib.c:(.text+0x4c7a): undefined reference to `ice_is_cgu_present' s390-linux-ld: ice_lib.c:(.text+0x4c96): undefined reference to `ice_is_clock_mux_present_e810t' sound/pci/hda/cirrus_scodec_test.c:151:60: error: initializer element is not a compile-time constant Unverified Error/Warning (likely false positive, please contact us if interested): Documentation/devicetree/bindings/mfd/qcom-pm8xxx.yaml: fs/ntfs3/bitmap.c:662 wnd_init() warn: Please consider using kvcalloc instead of kvmalloc_array fs/ntfs3/super.c:466:23: sparse: sparse: unknown escape sequence: '\%' Error/Warning ids grouped by kconfigs: gcc_recent_errors |-- arc-allmodconfig | |-- fs-bcachefs-bcachefs_format.h:warning:p-offset-in-struct-bkey-isn-t-aligned-to | `-- fs-bcachefs-bcachefs_format.h:warning:version-offset-in-struct-bkey-isn-t-aligned-to |-- arc-allyesconfig | |-- fs-bcachefs-bcachefs_format.h:warning:p-offset-in-struct-bkey-isn-t-aligned-to | `-- fs-bcachefs-bcachefs_format.h:warning:version-offset-in-struct-bkey-isn-t-aligned-to |-- arm-allmodconfig | `-- drivers-cpufreq-sti-cpufreq.c:warning:d-directive-output-may-be-truncated-writing-between-and-bytes-into-a-region-of-size |-- arm-allyesconfig | `-- drivers-cpufreq-sti-cpufreq.c:warning:d-directive-output-may-be-truncated-writing-between-and-bytes-into-a-region-of-size |-- arm64-allmodconfig | `-- include-linux-fortify-string.h:warning:writing-bytes-into-a-region-of-size |-- arm64-allyesconfig | `-- include-linux-fortify-string.h:warning:writing-bytes-into-a-region-of-size |-- i386-randconfig-006-20231004 | |-- drivers-net-ethernet-inte
Re: [PATCH] net/mlx5: Annotate struct mlx5_fc_bulk with __counted_by
On Tue, Oct 03, 2023 at 04:21:05PM -0700, Justin Stitt wrote: > On Tue, Oct 3, 2023 at 4:17 PM 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 mlx5_fc_bulk. > > > > Cc: Saeed Mahameed > > Cc: Leon Romanovsky > > Cc: "David S. Miller" > > Cc: Eric Dumazet > > Cc: Jakub Kicinski > > Cc: Paolo Abeni > > Cc: net...@vger.kernel.org > > Cc: linux-r...@vger.kernel.org > > Link: > > https://github.com/kees/kernel-tools/blob/trunk/coccinelle/examples/counted_by.cocci > > [1] > > Signed-off-by: Kees Cook > > --- > > drivers/net/ethernet/mellanox/mlx5/core/fs_counters.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > Thanks, Reviewed-by: Leon Romanovsky
Re: [PATCH] net/mlx5: Annotate struct mlx5_flow_handle with __counted_by
On Tue, Oct 03, 2023 at 04:17:30PM -0700, 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 mlx5_flow_handle. > > Cc: Saeed Mahameed > Cc: Leon Romanovsky > Cc: "David S. Miller" > Cc: Eric Dumazet > Cc: Jakub Kicinski > Cc: Paolo Abeni > Cc: net...@vger.kernel.org > Cc: linux-r...@vger.kernel.org > Link: > https://github.com/kees/kernel-tools/blob/trunk/coccinelle/examples/counted_by.cocci > [1] > Signed-off-by: Kees Cook > --- > drivers/net/ethernet/mellanox/mlx5/core/fs_core.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > Thanks, Reviewed-by: Leon Romanovsky
[PATCH v2 62/89] pstore: convert to new timestamp accessors
Convert to using the new inode timestamp accessor functions.
Signed-off-by: Jeff Layton
---
fs/pstore/inode.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/fs/pstore/inode.c b/fs/pstore/inode.c
index 585360706b33..d41c20d1b5e8 100644
--- a/fs/pstore/inode.c
+++ b/fs/pstore/inode.c
@@ -223,7 +223,7 @@ static struct inode *pstore_get_inode(struct super_block
*sb)
struct inode *inode = new_inode(sb);
if (inode) {
inode->i_ino = get_next_ino();
- inode->i_atime = inode->i_mtime =
inode_set_ctime_current(inode);
+ simple_inode_init_ts(inode);
}
return inode;
}
@@ -390,7 +390,8 @@ int pstore_mkfile(struct dentry *root, struct pstore_record
*record)
inode->i_private = private;
if (record->time.tv_sec)
- inode->i_mtime = inode_set_ctime_to_ts(inode, record->time);
+ inode_set_mtime_to_ts(inode,
+ inode_set_ctime_to_ts(inode,
record->time));
d_add(dentry, inode);
--
2.41.0
Re: [PATCH v2 62/89] pstore: convert to new timestamp accessors
On Wed, Oct 04, 2023 at 02:52:47PM -0400, Jeff Layton wrote: > Convert to using the new inode timestamp accessor functions. > > Signed-off-by: Jeff Layton Acked-by: Kees Cook -- Kees Cook
[PATCH] MAINTAINERS: Include additional ASoC paths
Make sure a few other paths are correctly sent to the ASoC maintainers. Link: https://lore.kernel.org/lkml/63dd3676.170a0220.1f1b2.3...@mx.google.com/ Cc: Mark Brown Signed-off-by: Kees Cook --- MAINTAINERS | 2 ++ 1 file changed, 2 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index 5f18ed0fbd42..585a13b9b52a 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -20116,6 +20116,8 @@ F: Documentation/devicetree/bindings/sound/ F: Documentation/sound/soc/ F: include/dt-bindings/sound/ F: include/sound/soc* +F: include/sound/sof/ +F: include/uapi/sound/asoc.h F: sound/soc/ SOUND - SOUND OPEN FIRMWARE (SOF) DRIVERS -- 2.34.1
Re: [PATCH] MAINTAINERS: Include additional ASoC paths
On Wed, Oct 04, 2023 at 12:34:45PM -0700, Kees Cook wrote: > +++ b/MAINTAINERS > @@ -20116,6 +20116,8 @@ F:Documentation/devicetree/bindings/sound/ > F: Documentation/sound/soc/ > F: include/dt-bindings/sound/ > F: include/sound/soc* > +F: include/sound/sof/ > +F: include/uapi/sound/asoc.h > F: sound/soc/ The SOF header is also missing from the entry for SOF. signature.asc Description: PGP signature
Re: [PATCH] MAINTAINERS: Include additional ASoC paths
On October 4, 2023 12:40:00 PM PDT, Mark Brown wrote: >On Wed, Oct 04, 2023 at 12:34:45PM -0700, Kees Cook wrote: > >> +++ b/MAINTAINERS >> @@ -20116,6 +20116,8 @@ F: Documentation/devicetree/bindings/sound/ >> F: Documentation/sound/soc/ >> F: include/dt-bindings/sound/ >> F: include/sound/soc* >> +F: include/sound/sof/ >> +F: include/uapi/sound/asoc.h >> F: sound/soc/ > >The SOF header is also missing from the entry for SOF. Ah, right! Can you take this and tweak it with the missing entries, or should I send a v2? -Kees -- Kees Cook
Re: [PATCH] MAINTAINERS: Include additional ASoC paths
On 10/4/23 21:34, Kees Cook wrote: Make sure a few other paths are correctly sent to the ASoC maintainers. Link: https://lore.kernel.org/lkml/63dd3676.170a0220.1f1b2.3...@mx.google.com/ Cc: Mark Brown Signed-off-by: Kees Cook Acked-by: Gustavo A. R. Silva Thanks! -- Gustavo --- MAINTAINERS | 2 ++ 1 file changed, 2 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index 5f18ed0fbd42..585a13b9b52a 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -20116,6 +20116,8 @@ F: Documentation/devicetree/bindings/sound/ F:Documentation/sound/soc/ F:include/dt-bindings/sound/ F:include/sound/soc* +F: include/sound/sof/ +F: include/uapi/sound/asoc.h F:sound/soc/ SOUND - SOUND OPEN FIRMWARE (SOF) DRIVERS
Re: [PATCH] MAINTAINERS: Include additional ASoC paths
On Wed, Oct 04, 2023 at 12:41:14PM -0700, Kees Cook wrote: > >The SOF header is also missing from the entry for SOF. > Ah, right! Can you take this and tweak it with the missing entries, or should > I send a v2? Could you send an incremental patch please? It's already in my CI as is. signature.asc Description: PGP signature
Re: [PATCH 0/5] chelsio: Annotate structs with __counted_by
Hello:
This series was applied to netdev/net-next.git (main)
by Jakub Kicinski :
On Fri, 29 Sep 2023 11:11:44 -0700 you wrote:
> Hi,
>
> This annotates several chelsio structures with the coming __counted_by
> attribute for bounds checking of flexible arrays at run-time. For more
> details,
> see commit dd06e72e68bc ("Compiler Attributes: Add __counted_by macro").
>
> Thanks!
>
> [...]
Here is the summary with links:
- [1/5] chelsio/l2t: Annotate struct l2t_data with __counted_by
https://git.kernel.org/netdev/net-next/c/3bbae5f1c651
- [2/5] cxgb4: Annotate struct clip_tbl with __counted_by
https://git.kernel.org/netdev/net-next/c/c3db467b0822
- [3/5] cxgb4: Annotate struct cxgb4_tc_u32_table with __counted_by
https://git.kernel.org/netdev/net-next/c/157c56a4fede
- [4/5] cxgb4: Annotate struct sched_table with __counted_by
https://git.kernel.org/netdev/net-next/c/ceba9725fb45
- [5/5] cxgb4: Annotate struct smt_data with __counted_by
https://git.kernel.org/netdev/net-next/c/1508cb7e0752
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
[PATCH] can: peak_pci: replace deprecated strncpy with strscpy
`strncpy` is deprecated for use on NUL-terminated destination strings [1] and as such we should prefer more robust and less ambiguous string interfaces. NUL-padding is not required since card is already zero-initialized: | card = kzalloc(sizeof(*card), GFP_KERNEL); A suitable replacement is `strscpy` [2] due to the fact that it guarantees NUL-termination on the destination buffer without unnecessarily NUL-padding. Link: https://www.kernel.org/doc/html/latest/process/deprecated.html#strncpy-on-nul-terminated-strings [1] Link: https://manpages.debian.org/testing/linux-manual-4.8/strscpy.9.en.html [2] Link: https://github.com/KSPP/linux/issues/90 Cc: linux-hardening@vger.kernel.org Signed-off-by: Justin Stitt --- Note: build-tested only. --- drivers/net/can/sja1000/peak_pci.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/can/sja1000/peak_pci.c b/drivers/net/can/sja1000/peak_pci.c index 84f34020aafb..da396d641e24 100644 --- a/drivers/net/can/sja1000/peak_pci.c +++ b/drivers/net/can/sja1000/peak_pci.c @@ -462,7 +462,7 @@ static int peak_pciec_probe(struct pci_dev *pdev, struct net_device *dev) card->led_chip.owner = THIS_MODULE; card->led_chip.dev.parent = &pdev->dev; card->led_chip.algo_data = &card->i2c_bit; - strncpy(card->led_chip.name, "peak_i2c", + strscpy(card->led_chip.name, "peak_i2c", sizeof(card->led_chip.name)); card->i2c_bit = peak_pciec_i2c_bit_ops; --- base-commit: cbf3a2cb156a2c911d8f38d8247814b4c07f49a2 change-id: 20231004-strncpy-drivers-net-can-sja1000-peak_pci-c-9c2e5e32be83 Best regards, -- Justin Stitt
[PATCH] net: dsa: lan9303: replace deprecated strncpy with memcpy
`strncpy` is deprecated for use on NUL-terminated destination strings
[1] and as such we should prefer more robust and less ambiguous
interfaces.
Let's opt for memcpy as we are copying strings into slices of length
`ETH_GSTRING_LEN` within the `data` buffer. Other similar get_strings()
implementations [2] [3] use memcpy().
Link:
https://www.kernel.org/doc/html/latest/process/deprecated.html#strncpy-on-nul-terminated-strings
[1]
Link:
https://elixir.bootlin.com/linux/v6.3/source/drivers/infiniband/ulp/opa_vnic/opa_vnic_ethtool.c#L167
[2]
Link:
https://elixir.bootlin.com/linux/v6.3/source/drivers/infiniband/ulp/ipoib/ipoib_ethtool.c#L137
[3]
Link: https://github.com/KSPP/linux/issues/90
Cc: linux-hardening@vger.kernel.org
Signed-off-by: Justin Stitt
---
Note: build-tested only.
---
drivers/net/dsa/lan9303-core.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/dsa/lan9303-core.c b/drivers/net/dsa/lan9303-core.c
index ee67adeb2cdb..665d69384b62 100644
--- a/drivers/net/dsa/lan9303-core.c
+++ b/drivers/net/dsa/lan9303-core.c
@@ -1013,8 +1013,8 @@ static void lan9303_get_strings(struct dsa_switch *ds,
int port,
return;
for (u = 0; u < ARRAY_SIZE(lan9303_mib); u++) {
- strncpy(data + u * ETH_GSTRING_LEN, lan9303_mib[u].name,
- ETH_GSTRING_LEN);
+ memcpy(data + u * ETH_GSTRING_LEN, lan9303_mib[u].name,
+ ETH_GSTRING_LEN);
}
}
---
base-commit: cbf3a2cb156a2c911d8f38d8247814b4c07f49a2
change-id: 20231005-strncpy-drivers-net-dsa-lan9303-core-c-6386858e5c22
Best regards,
--
Justin Stitt
[PATCH] net: ena: replace deprecated strncpy with strscpy
`strncpy` is deprecated for use on NUL-terminated destination strings [1] and as such we should prefer more robust and less ambiguous string interfaces. NUL-padding is not necessary as host_info is initialized to `ena_dev->host_attr.host_info` which is ultimately zero-initialized via alloc_etherdev_mq(). A suitable replacement is `strscpy` [2] due to the fact that it guarantees NUL-termination on the destination buffer without unnecessarily NUL-padding. Link: https://www.kernel.org/doc/html/latest/process/deprecated.html#strncpy-on-nul-terminated-strings [1] Link: https://manpages.debian.org/testing/linux-manual-4.8/strscpy.9.en.html [2] Link: https://github.com/KSPP/linux/issues/90 Cc: linux-hardening@vger.kernel.org Signed-off-by: Justin Stitt --- Note: build-tested only. --- drivers/net/ethernet/amazon/ena/ena_netdev.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/net/ethernet/amazon/ena/ena_netdev.c b/drivers/net/ethernet/amazon/ena/ena_netdev.c index f955bde10cf9..3118a617c9b6 100644 --- a/drivers/net/ethernet/amazon/ena/ena_netdev.c +++ b/drivers/net/ethernet/amazon/ena/ena_netdev.c @@ -3276,8 +3276,8 @@ static void ena_config_host_info(struct ena_com_dev *ena_dev, struct pci_dev *pd strscpy(host_info->kernel_ver_str, utsname()->version, sizeof(host_info->kernel_ver_str) - 1); host_info->os_dist = 0; - strncpy(host_info->os_dist_str, utsname()->release, - sizeof(host_info->os_dist_str) - 1); + strscpy(host_info->os_dist_str, utsname()->release, + sizeof(host_info->os_dist_str)); host_info->driver_version = (DRV_MODULE_GEN_MAJOR) | (DRV_MODULE_GEN_MINOR << ENA_ADMIN_HOST_INFO_MINOR_SHIFT) | --- base-commit: cbf3a2cb156a2c911d8f38d8247814b4c07f49a2 change-id: 20231005-strncpy-drivers-net-ethernet-amazon-ena-ena_netdev-c-6c4804466aa7 Best regards, -- Justin Stitt
[PATCH] net: ax88796c: replace deprecated strncpy with strscpy
`strncpy` is deprecated for use on NUL-terminated destination strings
[1] and as such we should prefer more robust and less ambiguous string
interfaces.
A suitable replacement is `strscpy` [2] due to the fact that it
guarantees NUL-termination on the destination buffer without
unnecessarily NUL-padding.
It should be noted that there doesn't currently exist a bug here as
DRV_NAME is a small string literal which means no overread bugs are
present.
Also to note, other ethernet drivers are using strscpy in a similar
pattern:
| dec/tulip/tulip_core.c
| 861:strscpy(info->driver, DRV_NAME, sizeof(info->driver));
|
| 8390/ax88796.c
| 582:strscpy(info->driver, DRV_NAME, sizeof(info->driver));
|
| dec/tulip/dmfe.c
| 1077: strscpy(info->driver, DRV_NAME, sizeof(info->driver));
|
| 8390/etherh.c
| 558:strscpy(info->driver, DRV_NAME, sizeof(info->driver));
Link:
https://www.kernel.org/doc/html/latest/process/deprecated.html#strncpy-on-nul-terminated-strings
[1]
Link: https://manpages.debian.org/testing/linux-manual-4.8/strscpy.9.en.html [2]
Link: https://github.com/KSPP/linux/issues/90
Cc: linux-hardening@vger.kernel.org
Signed-off-by: Justin Stitt
---
Note: build-tested only.
---
drivers/net/ethernet/asix/ax88796c_ioctl.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/asix/ax88796c_ioctl.c
b/drivers/net/ethernet/asix/ax88796c_ioctl.c
index 916ae380a004..7d2fe2e5af92 100644
--- a/drivers/net/ethernet/asix/ax88796c_ioctl.c
+++ b/drivers/net/ethernet/asix/ax88796c_ioctl.c
@@ -24,7 +24,7 @@ static void
ax88796c_get_drvinfo(struct net_device *ndev, struct ethtool_drvinfo *info)
{
/* Inherit standard device info */
- strncpy(info->driver, DRV_NAME, sizeof(info->driver));
+ strscpy(info->driver, DRV_NAME, sizeof(info->driver));
}
static u32 ax88796c_get_msglevel(struct net_device *ndev)
---
base-commit: cbf3a2cb156a2c911d8f38d8247814b4c07f49a2
change-id:
20231005-strncpy-drivers-net-ethernet-asix-ax88796c_ioctl-c-56af20b7d992
Best regards,
--
Justin Stitt
[PATCH] net: atheros: replace deprecated strncpy with strscpy
`strncpy` is deprecated for use on NUL-terminated destination strings [1] and as such we should prefer more robust and less ambiguous string interfaces. We expect netdev->name to be NUL-terminated based on its use with format strings and dev_info(): | dev_info(&adapter->pdev->dev, | "%s link is up %d Mbps %s\n", | netdev->name, adapter->link_speed, | adapter->link_duplex == FULL_DUPLEX ? | "full duplex" : "half duplex"); Furthermore, NUL-padding is not required as netdev is already zero-initialized through alloc_etherdev(). Considering the above, a suitable replacement is `strscpy` [2] due to the fact that it guarantees NUL-termination on the destination buffer without unnecessarily NUL-padding. Link: https://www.kernel.org/doc/html/latest/process/deprecated.html#strncpy-on-nul-terminated-strings [1] Link: https://manpages.debian.org/testing/linux-manual-4.8/strscpy.9.en.html [2] Link: https://github.com/KSPP/linux/issues/90 Cc: linux-hardening@vger.kernel.org Signed-off-by: Justin Stitt --- Note: build-tested only. --- drivers/net/ethernet/atheros/atlx/atl2.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/ethernet/atheros/atlx/atl2.c b/drivers/net/ethernet/atheros/atlx/atl2.c index 1b487c071cb6..bcfc9488125b 100644 --- a/drivers/net/ethernet/atheros/atlx/atl2.c +++ b/drivers/net/ethernet/atheros/atlx/atl2.c @@ -1377,7 +1377,7 @@ static int atl2_probe(struct pci_dev *pdev, const struct pci_device_id *ent) netdev->watchdog_timeo = 5 * HZ; netdev->min_mtu = 40; netdev->max_mtu = ETH_DATA_LEN + VLAN_HLEN; - strncpy(netdev->name, pci_name(pdev), sizeof(netdev->name) - 1); + strscpy(netdev->name, pci_name(pdev), sizeof(netdev->name)); netdev->mem_start = mmio_start; netdev->mem_end = mmio_start + mmio_len; --- base-commit: cbf3a2cb156a2c911d8f38d8247814b4c07f49a2 change-id: 20231005-strncpy-drivers-net-ethernet-atheros-atlx-atl2-c-d876945d6341 Best regards, -- Justin Stitt
[PATCH] MAINTAINERS: Include sof headers under ASoC
Add missing sof header files for ASoC. Suggested-by: Mark Brown Link: https://lore.kernel.org/lkml/f258a7e6-0728-4f55-a71a-6e99113ce...@sirena.org.uk Signed-off-by: Kees Cook --- MAINTAINERS | 2 ++ 1 file changed, 2 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index 585a13b9b52a..811e59d50291 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -20116,7 +20116,9 @@ F: Documentation/devicetree/bindings/sound/ F: Documentation/sound/soc/ F: include/dt-bindings/sound/ F: include/sound/soc* +F: include/sound/sof.h F: include/sound/sof/ +F: include/trace/events/sof*.h F: include/uapi/sound/asoc.h F: sound/soc/ -- 2.34.1
Re: [PATCH] can: peak_pci: replace deprecated strncpy with strscpy
On Thu, Oct 05, 2023 at 12:05:35AM +, Justin Stitt wrote: > `strncpy` is deprecated for use on NUL-terminated destination strings > [1] and as such we should prefer more robust and less ambiguous string > interfaces. > > NUL-padding is not required since card is already zero-initialized: > | card = kzalloc(sizeof(*card), GFP_KERNEL); > > A suitable replacement is `strscpy` [2] due to the fact that it > guarantees NUL-termination on the destination buffer without > unnecessarily NUL-padding. > > Link: > https://www.kernel.org/doc/html/latest/process/deprecated.html#strncpy-on-nul-terminated-strings > [1] > Link: https://manpages.debian.org/testing/linux-manual-4.8/strscpy.9.en.html > [2] > Link: https://github.com/KSPP/linux/issues/90 > Cc: linux-hardening@vger.kernel.org > Signed-off-by: Justin Stitt Yup, this looks like a standard direct replacement. Reviewed-by: Kees Cook -- Kees Cook
Re: [PATCH] net: dsa: lan9303: replace deprecated strncpy with memcpy
On Thu, Oct 05, 2023 at 12:30:18AM +, Justin Stitt wrote:
> `strncpy` is deprecated for use on NUL-terminated destination strings
> [1] and as such we should prefer more robust and less ambiguous
> interfaces.
>
> Let's opt for memcpy as we are copying strings into slices of length
> `ETH_GSTRING_LEN` within the `data` buffer. Other similar get_strings()
> implementations [2] [3] use memcpy().
>
> Link:
> https://www.kernel.org/doc/html/latest/process/deprecated.html#strncpy-on-nul-terminated-strings
> [1]
> Link:
> https://elixir.bootlin.com/linux/v6.3/source/drivers/infiniband/ulp/opa_vnic/opa_vnic_ethtool.c#L167
> [2]
> Link:
> https://elixir.bootlin.com/linux/v6.3/source/drivers/infiniband/ulp/ipoib/ipoib_ethtool.c#L137
> [3]
> Link: https://github.com/KSPP/linux/issues/90
> Cc: linux-hardening@vger.kernel.org
> Signed-off-by: Justin Stitt
> ---
> Note: build-tested only.
> ---
> drivers/net/dsa/lan9303-core.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/dsa/lan9303-core.c b/drivers/net/dsa/lan9303-core.c
> index ee67adeb2cdb..665d69384b62 100644
> --- a/drivers/net/dsa/lan9303-core.c
> +++ b/drivers/net/dsa/lan9303-core.c
> @@ -1013,8 +1013,8 @@ static void lan9303_get_strings(struct dsa_switch *ds,
> int port,
> return;
>
> for (u = 0; u < ARRAY_SIZE(lan9303_mib); u++) {
> - strncpy(data + u * ETH_GSTRING_LEN, lan9303_mib[u].name,
> - ETH_GSTRING_LEN);
> + memcpy(data + u * ETH_GSTRING_LEN, lan9303_mib[u].name,
> +ETH_GSTRING_LEN);
This won't work because lan9303_mib entries aren't ETH_GSTRING_LEN-long
strings; they're string pointers:
static const struct lan9303_mib_desc lan9303_mib[] = {
{ .offset = LAN9303_MAC_RX_BRDCST_CNT_0, .name = "RxBroad", },
So this really does need a strcpy-family function.
And, I think the vnic_gstrings_stats and ipoib_gstrings_stats examples
are actually buggy -- they're copying junk into userspace...
I am reminded of this patch, which correctly uses strscpy_pad():
https://lore.kernel.org/lkml/20230718-net-dsa-strncpy-v1-1-e84664747...@google.com/
I think you want to do the same here, and use strscpy_pad(). And perhaps
send some fixes for the other memcpy() users?
(Have I mentioned I really dislike the get_strings() API?)
-Kees
--
Kees Cook
Re: [PATCH] net: ena: replace deprecated strncpy with strscpy
On Thu, Oct 05, 2023 at 12:56:08AM +, Justin Stitt wrote: > `strncpy` is deprecated for use on NUL-terminated destination strings > [1] and as such we should prefer more robust and less ambiguous string > interfaces. > > NUL-padding is not necessary as host_info is initialized to > `ena_dev->host_attr.host_info` which is ultimately zero-initialized via > alloc_etherdev_mq(). > > A suitable replacement is `strscpy` [2] due to the fact that it > guarantees NUL-termination on the destination buffer without > unnecessarily NUL-padding. > > Link: > https://www.kernel.org/doc/html/latest/process/deprecated.html#strncpy-on-nul-terminated-strings > [1] > Link: https://manpages.debian.org/testing/linux-manual-4.8/strscpy.9.en.html > [2] > Link: https://github.com/KSPP/linux/issues/90 > Cc: linux-hardening@vger.kernel.org > Signed-off-by: Justin Stitt Looks right to me. Length nicely adjusted. :) Reviewed-by: Kees Cook -- Kees Cook
Re: [PATCH] net: ax88796c: replace deprecated strncpy with strscpy
On Thu, Oct 05, 2023 at 01:06:26AM +, Justin Stitt wrote: > `strncpy` is deprecated for use on NUL-terminated destination strings > [1] and as such we should prefer more robust and less ambiguous string > interfaces. > > A suitable replacement is `strscpy` [2] due to the fact that it > guarantees NUL-termination on the destination buffer without > unnecessarily NUL-padding. > > It should be noted that there doesn't currently exist a bug here as > DRV_NAME is a small string literal which means no overread bugs are > present. > > Also to note, other ethernet drivers are using strscpy in a similar > pattern: > | dec/tulip/tulip_core.c > | 861:strscpy(info->driver, DRV_NAME, sizeof(info->driver)); > | > | 8390/ax88796.c > | 582:strscpy(info->driver, DRV_NAME, sizeof(info->driver)); > | > | dec/tulip/dmfe.c > | 1077: strscpy(info->driver, DRV_NAME, sizeof(info->driver)); > | > | 8390/etherh.c > | 558:strscpy(info->driver, DRV_NAME, sizeof(info->driver)); > > Link: > https://www.kernel.org/doc/html/latest/process/deprecated.html#strncpy-on-nul-terminated-strings > [1] > Link: https://manpages.debian.org/testing/linux-manual-4.8/strscpy.9.en.html > [2] > Link: https://github.com/KSPP/linux/issues/90 > Cc: linux-hardening@vger.kernel.org > Signed-off-by: Justin Stitt Yeah, this looks like the others. Reviewed-by: Kees Cook -- Kees Cook
Re: [PATCH] net: atheros: replace deprecated strncpy with strscpy
On Thu, Oct 05, 2023 at 01:29:45AM +, Justin Stitt wrote: > `strncpy` is deprecated for use on NUL-terminated destination strings > [1] and as such we should prefer more robust and less ambiguous string > interfaces. > > We expect netdev->name to be NUL-terminated based on its use with format > strings and dev_info(): > | dev_info(&adapter->pdev->dev, > | "%s link is up %d Mbps %s\n", > | netdev->name, adapter->link_speed, > | adapter->link_duplex == FULL_DUPLEX ? > | "full duplex" : "half duplex"); > > Furthermore, NUL-padding is not required as netdev is already > zero-initialized through alloc_etherdev(). > > Considering the above, a suitable replacement is `strscpy` [2] due to > the fact that it guarantees NUL-termination on the destination buffer > without unnecessarily NUL-padding. > > Link: > https://www.kernel.org/doc/html/latest/process/deprecated.html#strncpy-on-nul-terminated-strings > [1] > Link: https://manpages.debian.org/testing/linux-manual-4.8/strscpy.9.en.html > [2] > Link: https://github.com/KSPP/linux/issues/90 > Cc: linux-hardening@vger.kernel.org > Signed-off-by: Justin Stitt Looks right; destination length correctly updated. Reviewed-by: Kees Cook -- Kees Cook
Re: [PATCH] net: dsa: lan9303: replace deprecated strncpy with memcpy
On Wed, Oct 04, 2023 at 08:07:55PM -0700, Kees Cook wrote:
> On Thu, Oct 05, 2023 at 12:30:18AM +, Justin Stitt wrote:
> > `strncpy` is deprecated for use on NUL-terminated destination strings
> > [1] and as such we should prefer more robust and less ambiguous
> > interfaces.
> >
> > Let's opt for memcpy as we are copying strings into slices of length
> > `ETH_GSTRING_LEN` within the `data` buffer. Other similar get_strings()
> > implementations [2] [3] use memcpy().
> >
> > Link:
> > https://www.kernel.org/doc/html/latest/process/deprecated.html#strncpy-on-nul-terminated-strings
> > [1]
> > Link:
> > https://elixir.bootlin.com/linux/v6.3/source/drivers/infiniband/ulp/opa_vnic/opa_vnic_ethtool.c#L167
> > [2]
> > Link:
> > https://elixir.bootlin.com/linux/v6.3/source/drivers/infiniband/ulp/ipoib/ipoib_ethtool.c#L137
> > [3]
> > Link: https://github.com/KSPP/linux/issues/90
> > Cc: linux-hardening@vger.kernel.org
> > Signed-off-by: Justin Stitt
> > ---
> > Note: build-tested only.
> > ---
> > drivers/net/dsa/lan9303-core.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/net/dsa/lan9303-core.c b/drivers/net/dsa/lan9303-core.c
> > index ee67adeb2cdb..665d69384b62 100644
> > --- a/drivers/net/dsa/lan9303-core.c
> > +++ b/drivers/net/dsa/lan9303-core.c
> > @@ -1013,8 +1013,8 @@ static void lan9303_get_strings(struct dsa_switch
> > *ds, int port,
> > return;
> >
> > for (u = 0; u < ARRAY_SIZE(lan9303_mib); u++) {
> > - strncpy(data + u * ETH_GSTRING_LEN, lan9303_mib[u].name,
> > - ETH_GSTRING_LEN);
> > + memcpy(data + u * ETH_GSTRING_LEN, lan9303_mib[u].name,
> > + ETH_GSTRING_LEN);
>
> This won't work because lan9303_mib entries aren't ETH_GSTRING_LEN-long
> strings; they're string pointers:
>
> static const struct lan9303_mib_desc lan9303_mib[] = {
> { .offset = LAN9303_MAC_RX_BRDCST_CNT_0, .name = "RxBroad", },
>
> So this really does need a strcpy-family function.
>
> And, I think the vnic_gstrings_stats and ipoib_gstrings_stats examples
> are actually buggy -- they're copying junk into userspace...
>
> I am reminded of this patch, which correctly uses strscpy_pad():
> https://lore.kernel.org/lkml/20230718-net-dsa-strncpy-v1-1-e84664747...@google.com/
>
> I think you want to do the same here, and use strscpy_pad(). And perhaps
> send some fixes for the other memcpy() users?
Meh, I think it's not worth fixing the memcpy() users of this. This
buggy pattern is very common, it seems:
$ git grep 'data.*ETH_GSTRING_LEN' | grep memcpy | wc -l
47
--
Kees Cook
