[PATCH v2] Input: axp20x-pek - avoid needless newline removal
This code is doing more work than it needs to.
Before handing off `val_str` to `kstrtouint()` we are eagerly removing
any trailing newline which requires copying `buf`, validating it's
length and checking/replacing any potential newlines.
kstrtouint() handles this implicitly:
kstrtouint ->
kstrotoull -> (documentation)
| /**
|* kstrtoull - convert a string to an unsigned long long
|* @s: The start of the string. The string must be null-terminated, and may
also
|* include a single newline before its terminating null. The first
character
|...
Let's remove the redundant functionality and let kstrtouint handle it.
Link: https://github.com/KSPP/linux/issues/90
Cc: linux-hardening@vger.kernel.org
Suggested-by: Kees Cook
Signed-off-by: Justin Stitt
---
Changes in v2:
- remove eager newline removal (thanks Kees)
- use better subject line (thanks Kees)
- rebase on 6465e260f4879080
- Link to v1:
https://lore.kernel.org/r/20230921-strncpy-drivers-input-misc-axp20x-pek-c-v1-1-f7f6f4a5c...@google.com
---
Note: build-tested only.
---
drivers/input/misc/axp20x-pek.c | 11 +--
1 file changed, 1 insertion(+), 10 deletions(-)
diff --git a/drivers/input/misc/axp20x-pek.c b/drivers/input/misc/axp20x-pek.c
index 4581606a28d6..24f9e9d893de 100644
--- a/drivers/input/misc/axp20x-pek.c
+++ b/drivers/input/misc/axp20x-pek.c
@@ -133,20 +133,11 @@ static ssize_t axp20x_store_attr(struct device *dev,
size_t count)
{
struct axp20x_pek *axp20x_pek = dev_get_drvdata(dev);
- char val_str[20];
- size_t len;
int ret, i;
unsigned int val, idx = 0;
unsigned int best_err = UINT_MAX;
- val_str[sizeof(val_str) - 1] = '\0';
- strncpy(val_str, buf, sizeof(val_str) - 1);
- len = strlen(val_str);
-
- if (len && val_str[len - 1] == '\n')
- val_str[len - 1] = '\0';
-
- ret = kstrtouint(val_str, 10, );
+ ret = kstrtouint(buf, 10, );
if (ret)
return ret;
---
base-commit: 6465e260f48790807eef06b583b38ca9789b6072
change-id: 20230921-strncpy-drivers-input-misc-axp20x-pek-c-3c4708924bbd
Best regards,
--
Justin Stitt
[PATCH v4] hwmon: (acpi_power_meter) replace open-coded kmemdup_nul
`strncpy` is deprecated for use on NUL-terminated destination strings [1].
Let's refactor this kcalloc() + strncpy() into a kmemdup_nul() which has
more obvious behavior and is less error prone.
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
Signed-off-by: Justin Stitt
---
Changes in v4:
- drop +1 from length arg (thanks Kees)
- reword subject line (thanks Kees)
- rebase onto 6465e260f4879080
- Link to v3:
https://lore.kernel.org/r/20230921-strncpy-drivers-hwmon-acpi_power_meter-c-v3-1-307552c6e...@google.com
Changes in v3:
- refactor to use kmemdup_nul() (thanks Thomas and Kees)
- change commit msg to reflect ^
- rebase onto 2cf0f71562387282
- Link to v2:
https://lore.kernel.org/r/20230919-strncpy-drivers-hwmon-acpi_power_meter-c-v2-1-8348432d6...@google.com
Changes in v2:
- use memcpy over strscpy (thanks Kees)
- Link to v1:
https://lore.kernel.org/r/20230914-strncpy-drivers-hwmon-acpi_power_meter-c-v1-1-905297479...@google.com
---
drivers/hwmon/acpi_power_meter.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/hwmon/acpi_power_meter.c b/drivers/hwmon/acpi_power_meter.c
index fa28d447f0df..c13b5c8a0433 100644
--- a/drivers/hwmon/acpi_power_meter.c
+++ b/drivers/hwmon/acpi_power_meter.c
@@ -796,14 +796,13 @@ static int read_capabilities(struct
acpi_power_meter_resource *resource)
goto error;
}
- *str = kcalloc(element->string.length + 1, sizeof(u8),
- GFP_KERNEL);
+ *str = kmemdup_nul(element->string.pointer,
element->string.length,
+GFP_KERNEL);
if (!*str) {
res = -ENOMEM;
goto error;
}
- strncpy(*str, element->string.pointer, element->string.length);
str++;
}
---
base-commit: 6465e260f48790807eef06b583b38ca9789b6072
change-id: 20230914-strncpy-drivers-hwmon-acpi_power_meter-c-c9f2d8053bef
Best regards,
--
Justin Stitt
Re: [PATCH] platform/chrome: wilco_ec: Annotate struct ec_event_queue with __counted_by
Hello: This patch was applied to chrome-platform/linux.git (for-kernelci) by Tzung-Bi Shih : On Fri, 22 Sep 2023 10:51:47 -0700 you 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 ec_event_queue. > > [...] Here is the summary with links: - platform/chrome: wilco_ec: Annotate struct ec_event_queue with __counted_by https://git.kernel.org/chrome-platform/c/1aa8df90f456 You are awesome, thank you! -- Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/patchwork/pwbot.html
Re: [PATCH] ocfs2: Annotate struct ocfs2_replay_map with __counted_by
On 9/23/23 1:49 AM, 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 ocfs2_replay_map.
>
> [1]
> https://github.com/kees/kernel-tools/blob/trunk/coccinelle/examples/counted_by.cocci
>
> Cc: Mark Fasheh
> Cc: Joel Becker
> Cc: Joseph Qi
> Cc: Nathan Chancellor
> Cc: Nick Desaulniers
> Cc: Tom Rix
> Cc: ocfs2-de...@lists.linux.dev
> Cc: l...@lists.linux.dev
> Signed-off-by: Kees Cook
Reviewed-by: Joseph Qi
> ---
> fs/ocfs2/journal.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/fs/ocfs2/journal.c b/fs/ocfs2/journal.c
> index ce215565d061..604fea3a26ff 100644
> --- a/fs/ocfs2/journal.c
> +++ b/fs/ocfs2/journal.c
> @@ -90,7 +90,7 @@ enum ocfs2_replay_state {
> struct ocfs2_replay_map {
> unsigned int rm_slots;
> enum ocfs2_replay_state rm_state;
> - unsigned char rm_replay_slots[];
> + unsigned char rm_replay_slots[] __counted_by(rm_slots);
> };
>
> static void ocfs2_replay_map_set_state(struct ocfs2_super *osb, int state)
Re: [PATCH] media: i2c: Annotate struct i2c_atr with __counted_by
On Fri, Sep 22, 2023 at 10:54:25AM -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 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 i2c_atr. > > [1] > https://github.com/kees/kernel-tools/blob/trunk/coccinelle/examples/counted_by.cocci > > Cc: Wolfram Sang > Cc: Tomi Valkeinen > Cc: Luca Ceresoli > Cc: Nathan Chancellor > Cc: Nick Desaulniers > Cc: Tom Rix > Cc: linux-...@vger.kernel.org > Cc: l...@lists.linux.dev > Signed-off-by: Kees Cook Removed "media: " from $subject and applied to for-next, thanks! signature.asc Description: PGP signature
Re: [PATCH] i2c: mux: demux-pinctrl: Annotate struct i2c_demux_pinctrl_priv with __counted_by
On Fri, Sep 22, 2023 at 10:49:59AM -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 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 > i2c_demux_pinctrl_priv. > 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: Wolfram Sang > Cc: Peter Rosin > Cc: linux-...@vger.kernel.org > Signed-off-by: Kees Cook Fixed a blank line and applied to for-next, thanks! signature.asc Description: PGP signature
Re: [GIT PULL] hardening fixes for v6.6-rc3
On Sun, 24 Sept 2023 at 09:58, Alexey Dobriyan wrote: > > Most of those in uapi/ are likely unnecessary: extern "C" means > "don't mangle", but kernel doesn't export functions to userspace > except vDSO so there is nothing to mangle in the first place. I suspect a lot of it is "this got copied-and-pasted from a source that used it". And even if you don't export, you have to *match* the linkage in case you have the same name. So I suspect that if you have any kind of prototype sharing between user space (that might use C++) and kernel space, and end up with the same helper functions in both cases, and having some header sharing, you end up with that pattern. And you do it just once, and then it spreads by copy-and-paste. And then about a third of the hits seem to be in tools, which is literally user space and probably actually has C and C++ mixing. Another third is the drm uapi files. I didn't even try to look at what the cause there is. But presumably there are common names used in user space vs kernel. And then the last third is random. We do have a few other uses of __cplusplus. Sometimes just "we have a structure member name that the C++ people decided was a magic keyword". So it's not like this new pattern is *completely* new - we've had random "people want to use this header with C++ compilers and that causes random issues" before. The details are different, the cause is similar. Linus
Re: [PATCH] scsi: target: tcmu: Annotate struct tcmu_tmr with __counted_by
On 22.09.23 19:53, 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 tcmu_tmr.
[1]
https://github.com/kees/kernel-tools/blob/trunk/coccinelle/examples/counted_by.cocci
Cc: Bodo Stroesser
Cc: "Martin K. Petersen"
Cc: linux-s...@vger.kernel.org
Cc: target-de...@vger.kernel.org
Signed-off-by: Kees Cook
---
drivers/target/target_core_user.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/target/target_core_user.c
b/drivers/target/target_core_user.c
index 22cc6cac0ba2..7eb94894bd68 100644
--- a/drivers/target/target_core_user.c
+++ b/drivers/target/target_core_user.c
@@ -201,7 +201,7 @@ struct tcmu_tmr {
uint8_t tmr_type;
uint32_t tmr_cmd_cnt;
- int16_t tmr_cmd_ids[];
+ int16_t tmr_cmd_ids[] __counted_by(tmr_cmd_cnt);
};
/*
Reviewed-by: Bodo Stroesser
Thank you,
Bodo
Re: [GIT PULL] hardening fixes for v6.6-rc3
On Sat, Sep 23, 2023 at 11:04:57AM -0700, Linus Torvalds wrote:
> On Fri, 22 Sept 2023 at 20:49, Kees Cook wrote:
> >
> > 2) __cplusplus is relatively common in UAPI headers already:
> >$ git grep __cplusplus -- include/uapi | wc -l
> >58
>
> Look a bit closer.
>
> Most of those - by far - is for the usual
>
> #if defined(__cplusplus)
> extern "C" {
> #endif
>
> pattern. IOW, it's explicitly not different code, but telling the C++
> compiler that "this is C code".
>
> So this new #ifdef is an ugly new pattern of "do totally different
> things for C++".
>
> Apparently required, but very ugly nonetheless.
Most of those in uapi/ are likely unnecessary: extern "C" means
"don't mangle", but kernel doesn't export functions to userspace
except vDSO so there is nothing to mangle in the first place.
Re: [PATCH] firewire: Annotate struct fw_node with __counted_by
Hi, On Fri, Sep 22, 2023 at 10:53:35AM -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 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 fw_node. > > [1] > https://github.com/kees/kernel-tools/blob/trunk/coccinelle/examples/counted_by.cocci > > Cc: Takashi Sakamoto > Cc: linux1394-de...@lists.sourceforge.net > Signed-off-by: Kees Cook > --- > drivers/firewire/core.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) Applied to for-next branch, thanks. Regards Takashi Sakamoto
Re: [PATCH] memory: atmel-ebi: Annotate struct atmel_ebi_dev with __counted_by
On Fri, 22 Sep 2023 10:52:15 -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 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 atmel_ebi_dev. > > [...] Applied, thanks! [1/1] memory: atmel-ebi: Annotate struct atmel_ebi_dev with __counted_by https://git.kernel.org/krzk/linux-mem-ctrl/c/c1f2c81631df7654c249a457b7a54bbc36042248 Best regards, -- Krzysztof Kozlowski
Re: [PATCH] platform/surface: aggregator: Annotate struct ssam_event with __counted_by
On 9/22/23 19:54, 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 ssam_event.
[1]
https://github.com/kees/kernel-tools/blob/trunk/coccinelle/examples/counted_by.cocci
Cc: Maximilian Luz
Cc: platform-driver-...@vger.kernel.org
Signed-off-by: Kees Cook
---
include/linux/surface_aggregator/controller.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/linux/surface_aggregator/controller.h
b/include/linux/surface_aggregator/controller.h
index cb7980805920..5b67f0f47d80 100644
--- a/include/linux/surface_aggregator/controller.h
+++ b/include/linux/surface_aggregator/controller.h
@@ -44,7 +44,7 @@ struct ssam_event {
u8 command_id;
u8 instance_id;
u16 length;
- u8 data[];
+ u8 data[] __counted_by(length);
};
/**
Thanks!
Reviewed-by: Maximilian Luz
Re: [PATCH] watch_queue: Annotate struct watch_filter with __counted_by
On Fri, 22 Sep 2023 10:54:08 -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 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 watch_filter. > > [...] Applied to the vfs.misc branch of the vfs/vfs.git tree. Patches in the vfs.misc branch should appear in linux-next soon. Please report any outstanding bugs that were missed during review in a new review to the original patch series allowing us to drop it. It's encouraged to provide Acked-bys and Reviewed-bys even though the patch has now been applied. If possible patch trailers will be updated. Note that commit hashes shown below are subject to change due to rebase, trailer updates or similar. If in doubt, please check the listed branch. tree: https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git branch: vfs.misc [1/1] watch_queue: Annotate struct watch_filter with __counted_by https://git.kernel.org/vfs/vfs/c/6b601adb5e79
Re: [PATCH 13/14] net: tulip: Annotate struct mediatable with __counted_by
On 9/22/23 11:28, 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 mediatable.
[1]
https://github.com/kees/kernel-tools/blob/trunk/coccinelle/examples/counted_by.cocci
Cc: "David S. Miller"
Cc: Eric Dumazet
Cc: Jakub Kicinski
Cc: Paolo Abeni
Cc: Shaokun Zhang
Cc: net...@vger.kernel.org
Cc: linux-par...@vger.kernel.org
Signed-off-by: Kees Cook
Reviewed-by: Gustavo A. R. Silva
Thanks
--
Gustavo
---
drivers/net/ethernet/dec/tulip/tulip.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/dec/tulip/tulip.h
b/drivers/net/ethernet/dec/tulip/tulip.h
index 0ed598dc7569..bd786dfbc066 100644
--- a/drivers/net/ethernet/dec/tulip/tulip.h
+++ b/drivers/net/ethernet/dec/tulip/tulip.h
@@ -381,7 +381,7 @@ struct mediatable {
unsigned has_reset:6;
u32 csr15dir;
u32 csr15val; /* 21143 NWay setting. */
- struct medialeaf mleaf[];
+ struct medialeaf mleaf[] __counted_by(leafcount);
};
