[PATCH 0/4] ACPI: sysfs: Fix some issues in create_of_modalias() and create_pnp_modalias()

[PATCH 0/4] ACPI: sysfs: Fix some issues in create_of_modalias() and create_pnp_modalias()

[Christophe JAILLET]

All issues have been introduced by the same commit, 8765c5ba1949 ("ACPI
/ scan: Rework modalias creation when "compatible" is present")

The first 2 patches fixe some issues related to string truncation checks
and to computation of the available space in the output buffer.

The 2 others are just some clean-ups.

Christophe JAILLET (4):
  ACPI: sysfs: Fix the check for a potential string truncation
  ACPI: sysfs: Fix a potential out-of-bound write in
    create_of_modalias()
  ACPI: sysfs: Remove some useless trailing NULL writes
  ACPI: sysfs: Remove some dead code

 drivers/acpi/device_sysfs.c | 18 ++++++++----------
 1 file changed, 8 insertions(+), 10 deletions(-)

-- 
2.32.0

[PATCH 0/4] ACPI: sysfs: Fix some issues in create_of_modalias() and create_pnp_modalias()

[Christophe JAILLET]

All issues have been introduced by the same commit, 8765c5ba1949 ("ACPI
/ scan: Rework modalias creation when "compatible" is present")

The first 2 patches fixe some issues related to string truncation checks
and to computation of the available space in the output buffer.

The 2 others are just some clean-ups.

Christophe JAILLET (4):
  ACPI: sysfs: Fix the check for a potential string truncation
  ACPI: sysfs: Fix a potential out-of-bound write in
    create_of_modalias()
  ACPI: sysfs: Remove some useless trailing NULL writes
  ACPI: sysfs: Remove some dead code

 drivers/acpi/device_sysfs.c | 18 ++++++++----------
 1 file changed, 8 insertions(+), 10 deletions(-)

-- 
2.32.0

[PATCH 1/4] ACPI: sysfs: Fix the check for a potential string truncation

[Christophe JAILLET]

snprintf() does not return negative values on error.
To know if the buffer was too small, the returned value should be compared
with the length of the passed buffer. If it is bigger or equal, then the
output has been truncated.

Update the test for truncation accordingly.

Also return -ENOMEM in such a case, as already done below in the same
functions.

Fixes: 8765c5ba1949 ("ACPI / scan: Rework modalias creation when "compatible" is present")
Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
---
 drivers/acpi/device_sysfs.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/acpi/device_sysfs.c b/drivers/acpi/device_sysfs.c
index 9d8e90744cb5..4deb36dccb73 100644
--- a/drivers/acpi/device_sysfs.c
+++ b/drivers/acpi/device_sysfs.c
@@ -158,8 +158,8 @@ static int create_pnp_modalias(const struct acpi_device *acpi_dev, char *modalia
 		return 0;
 
 	len = snprintf(modalias, size, "acpi:");
-	if (len <= 0)
-		return len;
+	if (len >= size)
+		return -ENOMEM;
 
 	size -= len;
 
@@ -212,8 +212,8 @@ static int create_of_modalias(const struct acpi_device *acpi_dev, char *modalias
 	len = snprintf(modalias, size, "of:N%sT", (char *)buf.pointer);
 	ACPI_FREE(buf.pointer);
 
-	if (len <= 0)
-		return len;
+	if (len >= size)
+		return -ENOMEM;
 
 	of_compatible = acpi_dev->data.of_compatible;
 	if (of_compatible->type == ACPI_TYPE_PACKAGE) {
-- 
2.32.0

[PATCH 2/4] ACPI: sysfs: Fix a potential out-of-bound write in create_of_modalias()

[Christophe JAILLET]

The remaining size of the buffer used by snprintf() is not updated after
the first write, so subsequent write in the 'for' loop a few lines below
can write some data past the end of the 'modalias' buffer.

Correctly update the remaining size.

Note that this pattern is already correctly written in
create_pnp_modalias().

Fixes: 8765c5ba1949 ("ACPI / scan: Rework modalias creation when "compatible" is present")
Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
---
 drivers/acpi/device_sysfs.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/acpi/device_sysfs.c b/drivers/acpi/device_sysfs.c
index 4deb36dccb73..7ec3142f3eda 100644
--- a/drivers/acpi/device_sysfs.c
+++ b/drivers/acpi/device_sysfs.c
@@ -215,6 +215,8 @@ static int create_of_modalias(const struct acpi_device *acpi_dev, char *modalias
 	if (len >= size)
 		return -ENOMEM;
 
+	size -= len;
+
 	of_compatible = acpi_dev->data.of_compatible;
 	if (of_compatible->type == ACPI_TYPE_PACKAGE) {
 		nval = of_compatible->package.count;
-- 
2.32.0

Re: [PATCH 2/4] ACPI: sysfs: Fix a potential out-of-bound write in create_of_modalias()

[Dan Carpenter]

On Mon, Oct 23, 2023 at 09:33:16PM +0200, Christophe JAILLET wrote:
> The remaining size of the buffer used by snprintf() is not updated after
> the first write, so subsequent write in the 'for' loop a few lines below
> can write some data past the end of the 'modalias' buffer.
> 
> Correctly update the remaining size.
> 
> Note that this pattern is already correctly written in
> create_pnp_modalias().
> 
> Fixes: 8765c5ba1949 ("ACPI / scan: Rework modalias creation when "compatible" is present")
> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
> ---
>  drivers/acpi/device_sysfs.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/acpi/device_sysfs.c b/drivers/acpi/device_sysfs.c
> index 4deb36dccb73..7ec3142f3eda 100644
> --- a/drivers/acpi/device_sysfs.c
> +++ b/drivers/acpi/device_sysfs.c
> @@ -215,6 +215,8 @@ static int create_of_modalias(const struct acpi_device *acpi_dev, char *modalias
>  	if (len >= size)
>  		return -ENOMEM;
>  
> +	size -= len;
> +

Yeah.  This is a good bugfix but it also shows why the canonical format
is better.  In the canonical format the "size - len" happens as part of
snprintf() instead of on a separate line where it can be forgotten.

	len += snprintf(buf + len, size - len, "string");

Also the user space version of snprintf() can fail but the
kernel space version can't.  This code is more complicated and introduces
a memory corruption bug because it is pretending that we need to check
for negatives.  People (someone) sometimes (once ten years ago) tell me
that checking for negatives is important for security but actually it's
the reverse.

regards,
dan carpenter

Re: [PATCH 2/4] ACPI: sysfs: Fix a potential out-of-bound write in create_of_modalias()

[Dan Carpenter]

[-- Attachment #1: Type: text/plain, Size: 2068 bytes --]

On Tue, Oct 24, 2023 at 08:55:25AM +0300, Dan Carpenter wrote:
> On Mon, Oct 23, 2023 at 09:33:16PM +0200, Christophe JAILLET wrote:
> > The remaining size of the buffer used by snprintf() is not updated after
> > the first write, so subsequent write in the 'for' loop a few lines below
> > can write some data past the end of the 'modalias' buffer.
> > 
> > Correctly update the remaining size.
> > 
> > Note that this pattern is already correctly written in
> > create_pnp_modalias().
> > 
> > Fixes: 8765c5ba1949 ("ACPI / scan: Rework modalias creation when "compatible" is present")
> > Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
> > ---
> >  drivers/acpi/device_sysfs.c | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/drivers/acpi/device_sysfs.c b/drivers/acpi/device_sysfs.c
> > index 4deb36dccb73..7ec3142f3eda 100644
> > --- a/drivers/acpi/device_sysfs.c
> > +++ b/drivers/acpi/device_sysfs.c
> > @@ -215,6 +215,8 @@ static int create_of_modalias(const struct acpi_device *acpi_dev, char *modalias
> >  	if (len >= size)
> >  		return -ENOMEM;
> >  
> > +	size -= len;
> > +
> 
> Yeah.  This is a good bugfix but it also shows why the canonical format
> is better.  In the canonical format the "size - len" happens as part of
> snprintf() instead of on a separate line where it can be forgotten.
> 
> 	len += snprintf(buf + len, size - len, "string");
> 
> Also the user space version of snprintf() can fail but the
> kernel space version can't.  This code is more complicated and introduces
> a memory corruption bug because it is pretending that we need to check
> for negatives.  People (someone) sometimes (once ten years ago) tell me
> that checking for negatives is important for security but actually it's
> the reverse.

So I had a Smatch check for this kind of stuff but it was pretty junk.
It also only looked for "modalias + len" and here the code is doing
"&modalias[len]".

I can fix it up a bit today and look again at the warnings.  Here is the
the check and the warnings as-is.

regards,
dan carpenter


[-- Attachment #2: check_snprintf_no_minus.c --]
[-- Type: text/x-csrc, Size: 1654 bytes --]

/*
 * Copyright (C) 2021 Oracle.
 *
 * This program is free software; you can redistribute it and/or
 * modify it under the terms of the GNU General Public License
 * as published by the Free Software Foundation; either version 2
 * of the License, or (at your option) any later version.
 *
 * This program is distributed in the hope that it will be useful,
 * but WITHOUT ANY WARRANTY; without even the implied warranty of
 * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
 * GNU General Public License for more details.
 *
 * You should have received a copy of the GNU General Public License
 * along with this program; if not, see http://www.gnu.org/copyleft/gpl.txt
 */

#include "smatch.h"
#include "smatch_slist.h"
#include "smatch_extra.h"

static int my_id;

static bool is_plus(struct expression *expr)
{
	if (expr->type == EXPR_BINOP && expr->op == '+')
		return true;
	/* fixme: what about dst += snprintf() */
	return false;
}

static void match_snprintf(const char *fn, struct expression *expr, void *unused)
{
	struct expression *dest, *limit;
	char *name;

	dest = get_argument_from_call_expr(expr->args, 0);
	limit = get_argument_from_call_expr(expr->args, 1);
	dest = strip_expr(dest);
	limit = strip_expr(limit);
	if (!dest || !limit)
		return;

	if (!is_plus(dest))
		return;

	if (limit->type == EXPR_BINOP && limit->op == '-')
		return;

	name = expr_to_str(limit);
	sm_warning("expected subtract in snprintf limit '%s'", name);
	free_string(name);
}

void check_snprintf_no_minus(int id)
{
	my_id = id;
	add_function_hook("snprintf", &match_snprintf, NULL);
	add_function_hook("scnprintf", &match_snprintf, NULL);
}


[-- Attachment #3: err-list --]
[-- Type: text/plain, Size: 11450 bytes --]

kernel/locking/lockdep_proc.c:497 seq_stats() warn: expected subtract in snprintf limit '3'
kernel/locking/lockdep_proc.c:501 seq_stats() warn: expected subtract in snprintf limit '3'
kernel/time/clocksource.c:1429 available_clocksource_show() warn: expected subtract in snprintf limit '(null)'
kernel/time/clocksource.c:1434 available_clocksource_show() warn: expected subtract in snprintf limit '(null)'
kernel/trace/trace_probe.c:1635 __set_print_fmt() warn: expected subtract in snprintf limit '(len) ?len - pos:0'
kernel/trace/trace_probe.c:1639 __set_print_fmt() warn: expected subtract in snprintf limit '(len) ?len - pos:0'
kernel/trace/trace_probe.c:1642 __set_print_fmt() warn: expected subtract in snprintf limit '(len) ?len - pos:0'
kernel/trace/trace_probe.c:1645 __set_print_fmt() warn: expected subtract in snprintf limit '(len) ?len - pos:0'
kernel/trace/trace_probe.c:1646 __set_print_fmt() warn: expected subtract in snprintf limit '(len) ?len - pos:0'
kernel/trace/trace_probe.c:1649 __set_print_fmt() warn: expected subtract in snprintf limit '(len) ?len - pos:0'
kernel/trace/trace_probe.c:1652 __set_print_fmt() warn: expected subtract in snprintf limit '(len) ?len - pos:0'
kernel/trace/trace_probe.c:1663 __set_print_fmt() warn: expected subtract in snprintf limit '(len) ?len - pos:0'
kernel/trace/trace_probe.c:1670 __set_print_fmt() warn: expected subtract in snprintf limit '(len) ?len - pos:0'
kernel/trace/trace_events_synth.c:616 __set_synth_event_print_fmt() warn: expected subtract in snprintf limit '(len) ?len - pos:0'
kernel/trace/trace_events_synth.c:619 __set_synth_event_print_fmt() warn: expected subtract in snprintf limit '(len) ?len - pos:0'
kernel/trace/trace_events_synth.c:623 __set_synth_event_print_fmt() warn: expected subtract in snprintf limit '(len) ?len - pos:0'
kernel/trace/trace_events_synth.c:628 __set_synth_event_print_fmt() warn: expected subtract in snprintf limit '(len) ?len - pos:0'
kernel/trace/trace_events_synth.c:631 __set_synth_event_print_fmt() warn: expected subtract in snprintf limit '(len) ?len - pos:0'
kernel/trace/trace_events_synth.c:634 __set_synth_event_print_fmt() warn: expected subtract in snprintf limit '(len) ?len - pos:0'
kernel/trace/trace_syscalls.c:218 __set_enter_print_fmt() warn: expected subtract in snprintf limit '(len) ?len - pos:0'
kernel/trace/trace_syscalls.c:220 __set_enter_print_fmt() warn: expected subtract in snprintf limit '(len) ?len - pos:0'
kernel/trace/trace_syscalls.c:224 __set_enter_print_fmt() warn: expected subtract in snprintf limit '(len) ?len - pos:0'
kernel/trace/trace_syscalls.c:227 __set_enter_print_fmt() warn: expected subtract in snprintf limit '(len) ?len - pos:0'
kernel/trace/trace_events_user.c:1330 user_dyn_field_set_string() warn: expected subtract in snprintf limit '(len) ?len - pos:0'
kernel/trace/trace_events_user.c:1332 user_dyn_field_set_string() warn: expected subtract in snprintf limit '(len) ?len - pos:0'
kernel/trace/trace_events_user.c:1353 user_field_set_string() warn: expected subtract in snprintf limit '(len) ?len - pos:0'
kernel/trace/trace_events_user.c:1354 user_field_set_string() warn: expected subtract in snprintf limit '(len) ?len - pos:0'
kernel/trace/trace_events_user.c:1355 user_field_set_string() warn: expected subtract in snprintf limit '(len) ?len - pos:0'
kernel/trace/trace_events_user.c:1358 user_field_set_string() warn: expected subtract in snprintf limit '(len) ?len - pos:0'
kernel/trace/trace_events_user.c:1361 user_field_set_string() warn: expected subtract in snprintf limit '(len) ?len - pos:0'
kernel/trace/trace_events_user.c:1373 user_event_set_print_fmt() warn: expected subtract in snprintf limit '(len) ?len - pos:0'
kernel/trace/trace_events_user.c:1377 user_event_set_print_fmt() warn: expected subtract in snprintf limit '(len) ?len - pos:0'
kernel/trace/trace_events_user.c:1379 user_event_set_print_fmt() warn: expected subtract in snprintf limit '(len) ?len - pos:0'
kernel/trace/trace_events_user.c:1385 user_event_set_print_fmt() warn: expected subtract in snprintf limit '(len) ?len - pos:0'
kernel/trace/trace_events_user.c:1390 user_event_set_print_fmt() warn: expected subtract in snprintf limit '(len) ?len - pos:0'
kernel/trace/trace_events_user.c:1393 user_event_set_print_fmt() warn: expected subtract in snprintf limit '(len) ?len - pos:0'
lib/ref_tracker.c:99 __ref_tracker_dir_pr_ostream() warn: expected subtract in snprintf limit 'len'
lib/ref_tracker.c:110 __ref_tracker_dir_pr_ostream() warn: expected subtract in snprintf limit 'len'
lib/ref_tracker.c:116 __ref_tracker_dir_pr_ostream() warn: expected subtract in snprintf limit 'len'
lib/dynamic_debug.c:828 __dynamic_emit_prefix() warn: expected subtract in snprintf limit 'remaining(pos)'
lib/dynamic_debug.c:830 __dynamic_emit_prefix() warn: expected subtract in snprintf limit 'remaining(pos)'
lib/dynamic_debug.c:836 __dynamic_emit_prefix() warn: expected subtract in snprintf limit 'remaining(pos)'
lib/dynamic_debug.c:839 __dynamic_emit_prefix() warn: expected subtract in snprintf limit 'remaining(pos)'
lib/dynamic_debug.c:841 __dynamic_emit_prefix() warn: expected subtract in snprintf limit 'remaining(pos)'
lib/dynamic_debug.c:845 __dynamic_emit_prefix() warn: expected subtract in snprintf limit 'remaining(pos)'
lib/dynamic_debug.c:847 __dynamic_emit_prefix() warn: expected subtract in snprintf limit 'remaining(pos)'
drivers/net/ethernet/broadcom/bnx2x/bnx2x_ethtool.c:3204 bnx2x_get_strings() warn: expected subtract in snprintf limit '32'
drivers/net/ethernet/sfc/siena/ethtool_common.c:229 efx_fill_test() warn: expected subtract in snprintf limit '32'
drivers/net/ethernet/sfc/falcon/ethtool.c:229 ef4_fill_test() warn: expected subtract in snprintf limit '32'
drivers/net/ethernet/sfc/ethtool_common.c:278 efx_fill_test() warn: expected subtract in snprintf limit '32'
drivers/net/ethernet/mellanox/mlx5/core/main.c:243 mlx5_set_driver_version() warn: expected subtract in snprintf limit 'remaining_size'
drivers/net/ethernet/mellanox/mlx5/core/lag/lag.c:198 mlx5_lag_print_mapping() warn: expected subtract in snprintf limit '4'
drivers/net/ethernet/mellanox/mlx5/core/lag/lag.c:209 mlx5_lag_print_mapping() warn: expected subtract in snprintf limit '10'
drivers/net/ethernet/mellanox/mlx4/eq.c:1264 mlx4_init_eq_table() warn: expected subtract in snprintf limit '32'
drivers/net/ethernet/mellanox/mlx4/eq.c:1505 mlx4_assign_eq() warn: expected subtract in snprintf limit '32'
drivers/net/ethernet/hisilicon/hns/hns_dsaf_main.c:2630 hns_dsaf_get_node_stats_strings() warn: expected subtract in snprintf limit '32'
drivers/net/ethernet/hisilicon/hns/hns_dsaf_main.c:2633 hns_dsaf_get_node_stats_strings() warn: expected subtract in snprintf limit '32'
drivers/net/ethernet/hisilicon/hns/hns_ethtool.c:436 __lb_other_process() warn: expected subtract in snprintf limit '3'
drivers/net/ethernet/qlogic/qede/qede_main.c:1205 qede_log_probe() warn: expected subtract in snprintf limit 'left_size'
drivers/ata/libata-transport.c:578 show_ata_dev_id() warn: expected subtract in snprintf limit '20'
drivers/ata/libata-transport.c:597 show_ata_dev_gscr() warn: expected subtract in snprintf limit '20'
drivers/usb/atm/cxacru.c:490 adsl_config_store() warn: expected subtract in snprintf limit '13'
drivers/gpu/drm/i915/display/intel_sdvo.c:423 intel_sdvo_debug_write() warn: expected subtract in snprintf limit '(null)'
drivers/gpu/drm/i915/display/intel_sdvo.c:426 intel_sdvo_debug_write() warn: expected subtract in snprintf limit '(null)'
drivers/gpu/drm/i915/display/intel_sdvo.c:431 intel_sdvo_debug_write() warn: expected subtract in snprintf limit '(null)'
drivers/gpu/drm/i915/display/intel_sdvo.c:433 intel_sdvo_debug_write() warn: expected subtract in snprintf limit '(null)'
drivers/gpu/drm/i915/display/intel_sdvo.c:586 intel_sdvo_read_response() warn: expected subtract in snprintf limit '(null)'
drivers/gpu/drm/i915/display/intel_sdvo.c:588 intel_sdvo_read_response() warn: expected subtract in snprintf limit '(null)'
drivers/gpu/drm/i915/display/intel_sdvo.c:599 intel_sdvo_read_response() warn: expected subtract in snprintf limit '(null)'
drivers/gpu/drm/gma500/psb_intel_sdvo.c:416 psb_intel_sdvo_debug_write() warn: expected subtract in snprintf limit '(null)'
drivers/gpu/drm/gma500/psb_intel_sdvo.c:420 psb_intel_sdvo_debug_write() warn: expected subtract in snprintf limit '(null)'
drivers/gpu/drm/gma500/psb_intel_sdvo.c:425 psb_intel_sdvo_debug_write() warn: expected subtract in snprintf limit '(null)'
drivers/gpu/drm/gma500/psb_intel_sdvo.c:431 psb_intel_sdvo_debug_write() warn: expected subtract in snprintf limit '(null)'
drivers/gpu/drm/gma500/psb_intel_sdvo.c:541 psb_intel_sdvo_read_response() warn: expected subtract in snprintf limit '(null)'
drivers/gpu/drm/gma500/psb_intel_sdvo.c:543 psb_intel_sdvo_read_response() warn: expected subtract in snprintf limit '(null)'
drivers/gpu/drm/gma500/psb_intel_sdvo.c:554 psb_intel_sdvo_read_response() warn: expected subtract in snprintf limit '(null)'
drivers/gpu/drm/drm_print.c:144 __drm_printfn_coredump() warn: expected subtract in snprintf limit 'iterator->remain'
drivers/md/dm-ima.c:337 dm_ima_measure_on_table_load() warn: expected subtract in snprintf limit '3'
drivers/input/input.c:1087 input_bits_to_string() warn: expected subtract in snprintf limit '(null)'
drivers/input/input.c:1386 input_print_modalias_bits() warn: expected subtract in snprintf limit '(null)'
drivers/input/input.c:1420 input_print_modalias() warn: expected subtract in snprintf limit '(null)'
drivers/input/input.c:1543 input_print_bitmap() warn: expected subtract in snprintf limit '(null)'
drivers/input/input.c:1554 input_print_bitmap() warn: expected subtract in snprintf limit '(null)'
drivers/input/misc/keyspan_remote.c:132 keyspan_print() warn: expected subtract in snprintf limit '4'
drivers/block/zram/zram_drv.c:878 read_block_state() warn: expected subtract in snprintf limit 'count'
drivers/infiniband/hw/cxgb4/device.c:267 dump_qp() warn: expected subtract in snprintf limit 'space'
drivers/infiniband/hw/cxgb4/device.c:288 dump_qp() warn: expected subtract in snprintf limit 'space'
drivers/infiniband/hw/cxgb4/device.c:304 dump_qp() warn: expected subtract in snprintf limit 'space'
drivers/infiniband/hw/cxgb4/device.c:390 dump_stag() warn: expected subtract in snprintf limit 'space'
drivers/infiniband/hw/cxgb4/device.c:574 dump_ep() warn: expected subtract in snprintf limit 'space'
drivers/infiniband/hw/cxgb4/device.c:595 dump_ep() warn: expected subtract in snprintf limit 'space'
drivers/infiniband/hw/cxgb4/device.c:631 dump_listen_ep() warn: expected subtract in snprintf limit 'space'
drivers/infiniband/hw/cxgb4/device.c:644 dump_listen_ep() warn: expected subtract in snprintf limit 'space'
drivers/accel/habanalabs/gaudi/gaudi.c:8927 gaudi_fill_sobs_from_mon() warn: expected subtract in snprintf limit 'max_write'
drivers/accel/habanalabs/gaudi/gaudi.c:8930 gaudi_fill_sobs_from_mon() warn: expected subtract in snprintf limit 'max_write'
fs/ocfs2/filecheck.c:327 ocfs2_filecheck_attr_show() warn: expected subtract in snprintf limit 'remain'
fs/ecryptfs/crypto.c:127 ecryptfs_derive_iv() warn: expected subtract in snprintf limit '16'
sound/soc/sof/sof-client-probes.c:226 sof_probes_dfs_points_read() warn: expected subtract in snprintf limit 'remaining'
sound/soc/sof/ipc4-mtrace.c:320 sof_ipc4_priority_mask_dfs_read() warn: expected subtract in snprintf limit 'remaining'

Re: [PATCH 2/4] ACPI: sysfs: Fix a potential out-of-bound write in create_of_modalias()

[Dan Carpenter]

[-- Attachment #1: Type: text/plain, Size: 404 bytes --]

On Tue, Oct 24, 2023 at 09:08:26AM +0300, Dan Carpenter wrote:
> So I had a Smatch check for this kind of stuff but it was pretty junk.
> It also only looked for "modalias + len" and here the code is doing
> "&modalias[len]".
> 
> I can fix it up a bit today and look again at the warnings.  Here is the
> the check and the warnings as-is.

Alright.  Here is the new version.  :)

regards,
dan carpenter

[-- Attachment #2: check_snprintf_no_minus.c --]
[-- Type: text/x-csrc, Size: 3152 bytes --]

/*
 * Copyright (C) 2021 Oracle.
 *
 * This program is free software; you can redistribute it and/or
 * modify it under the terms of the GNU General Public License
 * as published by the Free Software Foundation; either version 2
 * of the License, or (at your option) any later version.
 *
 * This program is distributed in the hope that it will be useful,
 * but WITHOUT ANY WARRANTY; without even the implied warranty of
 * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
 * GNU General Public License for more details.
 *
 * You should have received a copy of the GNU General Public License
 * along with this program; if not, see http://www.gnu.org/copyleft/gpl.txt
 */

#include "smatch.h"
#include "smatch_slist.h"
#include "smatch_extra.h"

static int my_id;

STATE(plus_equal);
STATE(minus_equal);

static bool is_plus_address(struct expression *expr)
{
	sval_t sval;

	if (expr->type != EXPR_PREOP ||
	    expr->op != '&')
		return false;

	expr = expr->unop;
	if (expr->type != EXPR_PREOP ||
	    expr->op != '*')
		return false;

	expr = expr->unop;
	if (expr->type != EXPR_BINOP ||
	    expr->op != '+')
		return false;

	if (get_implied_value(expr->right, &sval) && sval.value == 0)
		return false;

	return true;
}

static bool is_plus(struct expression *expr)
{
	struct expression *tmp;
	sval_t sval;

	tmp = get_assigned_expr(expr);
	if (tmp)
		expr = tmp;
	expr = strip_expr(expr);
	if (expr->type == EXPR_BINOP && expr->op == '+') {
		if (get_implied_value(expr->right, &sval) && sval.value == 0)
			return false;
		return true;
	}

	if (is_plus_address(expr))
		return true;

	if (get_state_expr(my_id, expr) == &plus_equal)
		return true;

	return false;
}

static bool is_minus(struct expression *expr)
{
	struct expression *tmp;

	tmp = get_assigned_expr(expr);
	if (tmp)
		expr = tmp;
	expr = strip_expr(expr);
	if (expr->type == EXPR_BINOP && expr->op == '-')
		return true;
	/*
	 * If, after calling strip_expr, the expression is still () that means
	 * it is an EXPR_STATEMENT and some kind of complicated macro.
	 */
	if (expr->type == EXPR_PREOP && expr->op == '(')
		return true;
	if (get_state_expr(my_id, expr) == &minus_equal)
		return true;
	return false;
}

static void match_snprintf(const char *fn, struct expression *expr, void *unused)
{
	struct expression *dest, *limit;
	char *name;

	dest = get_argument_from_call_expr(expr->args, 0);
	limit = get_argument_from_call_expr(expr->args, 1);
	dest = strip_expr(dest);
	limit = strip_expr(limit);
	if (!dest || !limit)
		return;

	if (!is_plus(dest))
		return;

	if (is_minus(limit))
		return;

	name = expr_to_str(limit);
	sm_warning("expected subtract in snprintf limit '%s'", name);
	free_string(name);
}

static void match_assign(struct expression *expr)
{
	if (expr->op == SPECIAL_ADD_ASSIGN)
		set_state_expr(my_id, expr->left, &plus_equal);

	if (expr->op == SPECIAL_SUB_ASSIGN)
		set_state_expr(my_id, expr->left, &minus_equal);
}

void check_snprintf_no_minus(int id)
{
	my_id = id;

	add_function_hook("snprintf", &match_snprintf, NULL);
	add_function_hook("scnprintf", &match_snprintf, NULL);

	add_hook(&match_assign, ASSIGNMENT_HOOK);
}

Re: [PATCH 2/4] ACPI: sysfs: Fix a potential out-of-bound write in create_of_modalias()

[Dan Carpenter]

[-- Attachment #1: Type: text/plain, Size: 1319 bytes --]

On Tue, Oct 24, 2023 at 10:09:31AM +0300, Dan Carpenter wrote:
> On Tue, Oct 24, 2023 at 09:08:26AM +0300, Dan Carpenter wrote:
> > So I had a Smatch check for this kind of stuff but it was pretty junk.
> > It also only looked for "modalias + len" and here the code is doing
> > "&modalias[len]".
> > 
> > I can fix it up a bit today and look again at the warnings.  Here is the
> > the check and the warnings as-is.
> 
> Alright.  Here is the new version.  :)
> 

Here are the results.  The only new real bugs I found were:

drivers/edac/i5400_edac.c:1036 calculate_dimm_size() warn: expected subtract in snprintf limit 'space'
drivers/accel/habanalabs/gaudi/gaudi.c:8930 gaudi_fill_sobs_from_mon() warn: expected subtract in snprintf limit 'max_write'

These ones are not bugs, but they're pretty messed up...  (It's debugfs
code so no one cares).
drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm_debugfs.c:217 dp_link_settings_read() warn: expected subtract in snprintf limit 'str_len'
drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm_debugfs.c:224 dp_link_settings_read() warn: expected subtract in snprintf limit 'str_len'
drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm_debugfs.c:231 dp_link_settings_read() warn: expected subtract in snprintf limit 'str_len'

regards,
dan carpenter


[-- Attachment #2: err-list --]
[-- Type: text/plain, Size: 15935 bytes --]

kernel/locking/lockdep_proc.c:497 seq_stats() warn: expected subtract in snprintf limit '3'
kernel/locking/lockdep_proc.c:501 seq_stats() warn: expected subtract in snprintf limit '3'
kernel/trace/trace_probe.c:1642 __set_print_fmt() warn: expected subtract in snprintf limit '(len) ?len - pos:0'
kernel/trace/trace_probe.c:1645 __set_print_fmt() warn: expected subtract in snprintf limit '(len) ?len - pos:0'
kernel/trace/trace_probe.c:1646 __set_print_fmt() warn: expected subtract in snprintf limit '(len) ?len - pos:0'
kernel/trace/trace_probe.c:1649 __set_print_fmt() warn: expected subtract in snprintf limit '(len) ?len - pos:0'
kernel/trace/trace_probe.c:1652 __set_print_fmt() warn: expected subtract in snprintf limit '(len) ?len - pos:0'
kernel/trace/trace_probe.c:1663 __set_print_fmt() warn: expected subtract in snprintf limit '(len) ?len - pos:0'
kernel/trace/trace_probe.c:1670 __set_print_fmt() warn: expected subtract in snprintf limit '(len) ?len - pos:0'
kernel/trace/trace_events_synth.c:619 __set_synth_event_print_fmt() warn: expected subtract in snprintf limit '(len) ?len - pos:0'
kernel/trace/trace_events_synth.c:623 __set_synth_event_print_fmt() warn: expected subtract in snprintf limit '(len) ?len - pos:0'
kernel/trace/trace_events_synth.c:628 __set_synth_event_print_fmt() warn: expected subtract in snprintf limit '(len) ?len - pos:0'
kernel/trace/trace_events_synth.c:631 __set_synth_event_print_fmt() warn: expected subtract in snprintf limit '(len) ?len - pos:0'
kernel/trace/trace_events_synth.c:634 __set_synth_event_print_fmt() warn: expected subtract in snprintf limit '(len) ?len - pos:0'
kernel/trace/trace_syscalls.c:220 __set_enter_print_fmt() warn: expected subtract in snprintf limit '(len) ?len - pos:0'
kernel/trace/trace_syscalls.c:224 __set_enter_print_fmt() warn: expected subtract in snprintf limit '(len) ?len - pos:0'
kernel/trace/trace_syscalls.c:227 __set_enter_print_fmt() warn: expected subtract in snprintf limit '(len) ?len - pos:0'
kernel/trace/trace_events_user.c:1332 user_dyn_field_set_string() warn: expected subtract in snprintf limit '(len) ?len - pos:0'
kernel/trace/trace_events_user.c:1354 user_field_set_string() warn: expected subtract in snprintf limit '(len) ?len - pos:0'
kernel/trace/trace_events_user.c:1355 user_field_set_string() warn: expected subtract in snprintf limit '(len) ?len - pos:0'
kernel/trace/trace_events_user.c:1358 user_field_set_string() warn: expected subtract in snprintf limit '(len) ?len - pos:0'
kernel/trace/trace_events_user.c:1361 user_field_set_string() warn: expected subtract in snprintf limit '(len) ?len - pos:0'
kernel/trace/trace_events_user.c:1377 user_event_set_print_fmt() warn: expected subtract in snprintf limit '(len) ?len - pos:0'
kernel/trace/trace_events_user.c:1379 user_event_set_print_fmt() warn: expected subtract in snprintf limit '(len) ?len - pos:0'
kernel/trace/trace_events_user.c:1385 user_event_set_print_fmt() warn: expected subtract in snprintf limit '(len) ?len - pos:0'
kernel/trace/trace_events_user.c:1390 user_event_set_print_fmt() warn: expected subtract in snprintf limit '(len) ?len - pos:0'
kernel/trace/trace_events_user.c:1393 user_event_set_print_fmt() warn: expected subtract in snprintf limit '(len) ?len - pos:0'
arch/x86/kernel/apic/io_apic.c:2669 ioapic_setup_resources() warn: expected subtract in snprintf limit '11'
arch/x86/kernel/acpi/boot.c:1006 acpi_parse_hpet() warn: expected subtract in snprintf limit '9'
lib/dynamic_debug.c:836 __dynamic_emit_prefix() warn: expected subtract in snprintf limit 'remaining(pos)'
lib/dynamic_debug.c:839 __dynamic_emit_prefix() warn: expected subtract in snprintf limit 'remaining(pos)'
lib/dynamic_debug.c:841 __dynamic_emit_prefix() warn: expected subtract in snprintf limit 'remaining(pos)'
lib/dynamic_debug.c:845 __dynamic_emit_prefix() warn: expected subtract in snprintf limit 'remaining(pos)'
lib/dynamic_debug.c:847 __dynamic_emit_prefix() warn: expected subtract in snprintf limit 'remaining(pos)'
drivers/hid/hid-lg4ff.c:1417 lg4ff_init() warn: expected subtract in snprintf limit 'name_sz'
drivers/hid/hid-sony.c:1438 sony_leds_init() warn: expected subtract in snprintf limit 'name_sz'
drivers/hid/hid-sony.c:1441 sony_leds_init() warn: expected subtract in snprintf limit 'name_sz'
drivers/hid/hid-picolcd_leds.c:120 picolcd_init_leds() warn: expected subtract in snprintf limit 'name_sz'
drivers/hid/hid-wiimote-modules.c:353 wiimod_led_probe() warn: expected subtract in snprintf limit 'namesz'
drivers/hid/hid-steelseries.c:296 steelseries_srws1_probe() warn: expected subtract in snprintf limit 'name_sz'
drivers/hid/hid-steelseries.c:317 steelseries_srws1_probe() warn: expected subtract in snprintf limit 'name_sz'
drivers/hid/hid-bigbenff.c:435 bigben_probe() warn: expected subtract in snprintf limit 'name_sz'
drivers/net/ethernet/engleder/tsnep_ethtool.c:147 tsnep_ethtool_get_strings() warn: expected subtract in snprintf limit '32'
drivers/net/ethernet/engleder/tsnep_ethtool.c:155 tsnep_ethtool_get_strings() warn: expected subtract in snprintf limit '32'
drivers/net/ethernet/broadcom/bnx2x/bnx2x_ethtool.c:3204 bnx2x_get_strings() warn: expected subtract in snprintf limit '32'
drivers/net/ethernet/chelsio/cxgb4/sge.c:2721 cxgb4_selftest_lb_pkt() warn: expected subtract in snprintf limit '17'
drivers/net/ethernet/sfc/siena/ethtool_common.c:229 efx_fill_test() warn: expected subtract in snprintf limit '32'
drivers/net/ethernet/sfc/falcon/ethtool.c:229 ef4_fill_test() warn: expected subtract in snprintf limit '32'
drivers/net/ethernet/sfc/ethtool_common.c:278 efx_fill_test() warn: expected subtract in snprintf limit '32'
drivers/net/ethernet/mellanox/mlx5/core/main.c:243 mlx5_set_driver_version() warn: expected subtract in snprintf limit 'remaining_size'
drivers/net/ethernet/mellanox/mlx4/eq.c:1505 mlx4_assign_eq() warn: expected subtract in snprintf limit '32'
drivers/net/ethernet/hisilicon/hns3/hns3_common/hclge_comm_tqp_stats.c:53 hclge_comm_tqps_get_strings() warn: expected subtract in snprintf limit '32'
drivers/net/ethernet/hisilicon/hns/hns_dsaf_main.c:2602 hns_dsaf_get_node_stats_strings() warn: expected subtract in snprintf limit '32'
drivers/net/ethernet/hisilicon/hns/hns_dsaf_main.c:2604 hns_dsaf_get_node_stats_strings() warn: expected subtract in snprintf limit '32'
drivers/net/ethernet/hisilicon/hns/hns_dsaf_main.c:2606 hns_dsaf_get_node_stats_strings() warn: expected subtract in snprintf limit '32'
drivers/net/ethernet/hisilicon/hns/hns_dsaf_main.c:2608 hns_dsaf_get_node_stats_strings() warn: expected subtract in snprintf limit '32'
drivers/net/ethernet/hisilicon/hns/hns_dsaf_main.c:2610 hns_dsaf_get_node_stats_strings() warn: expected subtract in snprintf limit '32'
drivers/net/ethernet/hisilicon/hns/hns_dsaf_main.c:2612 hns_dsaf_get_node_stats_strings() warn: expected subtract in snprintf limit '32'
drivers/net/ethernet/hisilicon/hns/hns_dsaf_main.c:2614 hns_dsaf_get_node_stats_strings() warn: expected subtract in snprintf limit '32'
drivers/net/ethernet/hisilicon/hns/hns_dsaf_main.c:2616 hns_dsaf_get_node_stats_strings() warn: expected subtract in snprintf limit '32'
drivers/net/ethernet/hisilicon/hns/hns_dsaf_main.c:2618 hns_dsaf_get_node_stats_strings() warn: expected subtract in snprintf limit '32'
drivers/net/ethernet/hisilicon/hns/hns_dsaf_main.c:2620 hns_dsaf_get_node_stats_strings() warn: expected subtract in snprintf limit '32'
drivers/net/ethernet/hisilicon/hns/hns_dsaf_main.c:2622 hns_dsaf_get_node_stats_strings() warn: expected subtract in snprintf limit '32'
drivers/net/ethernet/hisilicon/hns/hns_dsaf_main.c:2624 hns_dsaf_get_node_stats_strings() warn: expected subtract in snprintf limit '32'
drivers/net/ethernet/hisilicon/hns/hns_dsaf_main.c:2633 hns_dsaf_get_node_stats_strings() warn: expected subtract in snprintf limit '32'
drivers/net/ethernet/hisilicon/hns/hns_dsaf_main.c:2638 hns_dsaf_get_node_stats_strings() warn: expected subtract in snprintf limit '32'
drivers/net/ethernet/hisilicon/hns/hns_ethtool.c:436 __lb_other_process() warn: expected subtract in snprintf limit '3'
drivers/net/ethernet/cavium/liquidio/lio_core.c:1101 octeon_setup_interrupt() warn: expected subtract in snprintf limit '32'
drivers/net/ethernet/cavium/liquidio/lio_core.c:1122 octeon_setup_interrupt() warn: expected subtract in snprintf limit '32'
drivers/net/ethernet/cavium/liquidio/lio_core.c:1127 octeon_setup_interrupt() warn: expected subtract in snprintf limit '32'
drivers/net/ethernet/freescale/enetc/enetc_ethtool.c:262 enetc_get_strings() warn: expected subtract in snprintf limit '32'
drivers/net/ethernet/freescale/enetc/enetc_ethtool.c:269 enetc_get_strings() warn: expected subtract in snprintf limit '32'
drivers/net/ethernet/google/gve/gve_ethtool.c:106 gve_get_strings() warn: expected subtract in snprintf limit '32'
drivers/net/ethernet/google/gve/gve_ethtool.c:114 gve_get_strings() warn: expected subtract in snprintf limit '32'
drivers/net/ethernet/aquantia/atlantic/aq_ethtool.c:281 aq_ethtool_get_strings() warn: expected subtract in snprintf limit '32'
drivers/net/ethernet/aquantia/atlantic/aq_ethtool.c:288 aq_ethtool_get_strings() warn: expected subtract in snprintf limit '32'
drivers/net/ethernet/aquantia/atlantic/aq_ethtool.c:316 aq_ethtool_get_strings() warn: expected subtract in snprintf limit '32'
drivers/net/ethernet/aquantia/atlantic/aq_ethtool.c:341 aq_ethtool_get_strings() warn: expected subtract in snprintf limit '32'
drivers/net/ethernet/aquantia/atlantic/aq_ethtool.c:353 aq_ethtool_get_strings() warn: expected subtract in snprintf limit '32'
drivers/net/ethernet/marvell/octeon_ep/octep_main.c:358 octep_request_irqs() warn: expected subtract in snprintf limit '16 + 32'
drivers/net/ethernet/marvell/octeon_ep/octep_ethtool.c:97 octep_get_strings() warn: expected subtract in snprintf limit '32'
drivers/net/ethernet/marvell/octeon_ep/octep_ethtool.c:105 octep_get_strings() warn: expected subtract in snprintf limit '32'
drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c:1971 mvpp2_ethtool_get_strings() warn: expected subtract in snprintf limit '32'
drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c:1980 mvpp2_ethtool_get_strings() warn: expected subtract in snprintf limit '32'
drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c:207 otx2_register_flr_me_intr() warn: expected subtract in snprintf limit '32'
drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c:228 otx2_register_flr_me_intr() warn: expected subtract in snprintf limit '32'
drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c:238 otx2_register_flr_me_intr() warn: expected subtract in snprintf limit '32'
drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c:707 otx2_register_pfvf_mbox_intr() warn: expected subtract in snprintf limit '32'
drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c:710 otx2_register_pfvf_mbox_intr() warn: expected subtract in snprintf limit '32'
drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c:723 otx2_register_pfvf_mbox_intr() warn: expected subtract in snprintf limit '32'
drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c:726 otx2_register_pfvf_mbox_intr() warn: expected subtract in snprintf limit '32'
drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c:1004 otx2_register_mbox_intr() warn: expected subtract in snprintf limit '32'
drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c:1790 otx2_open() warn: expected subtract in snprintf limit '32'
drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c:1810 otx2_open() warn: expected subtract in snprintf limit '32'
drivers/net/can/can327.c:629 can327_handle_prompt() warn: expected subtract in snprintf limit '18'
drivers/net/can/can327.c:634 can327_handle_prompt() warn: expected subtract in snprintf limit '18'
drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c:385 brcmf_fw_add_macaddr() warn: expected subtract in snprintf limit '(7 + 6 * 3) + 1'
drivers/edac/i5400_edac.c:1036 calculate_dimm_size() warn: expected subtract in snprintf limit 'space'
drivers/usb/atm/cxacru.c:490 adsl_config_store() warn: expected subtract in snprintf limit '13'
drivers/media/test-drivers/vivid/vivid-core.c:1283 vivid_init_dv_timings() warn: expected subtract in snprintf limit '32'
drivers/iommu/omap-iommu-debug.c:49 omap2_iommu_dump_ctx() warn: expected subtract in snprintf limit 'maxcol'
drivers/iommu/omap-iommu-debug.c:50 omap2_iommu_dump_ctx() warn: expected subtract in snprintf limit 'maxcol'
drivers/iommu/omap-iommu-debug.c:51 omap2_iommu_dump_ctx() warn: expected subtract in snprintf limit 'maxcol'
drivers/iommu/omap-iommu-debug.c:52 omap2_iommu_dump_ctx() warn: expected subtract in snprintf limit 'maxcol'
drivers/iommu/omap-iommu-debug.c:53 omap2_iommu_dump_ctx() warn: expected subtract in snprintf limit 'maxcol'
drivers/iommu/omap-iommu-debug.c:54 omap2_iommu_dump_ctx() warn: expected subtract in snprintf limit 'maxcol'
drivers/iommu/omap-iommu-debug.c:55 omap2_iommu_dump_ctx() warn: expected subtract in snprintf limit 'maxcol'
drivers/iommu/omap-iommu-debug.c:56 omap2_iommu_dump_ctx() warn: expected subtract in snprintf limit 'maxcol'
drivers/iommu/omap-iommu-debug.c:57 omap2_iommu_dump_ctx() warn: expected subtract in snprintf limit 'maxcol'
drivers/iommu/omap-iommu-debug.c:58 omap2_iommu_dump_ctx() warn: expected subtract in snprintf limit 'maxcol'
drivers/iommu/omap-iommu-debug.c:59 omap2_iommu_dump_ctx() warn: expected subtract in snprintf limit 'maxcol'
drivers/iommu/omap-iommu-debug.c:60 omap2_iommu_dump_ctx() warn: expected subtract in snprintf limit 'maxcol'
drivers/iommu/omap-iommu-debug.c:61 omap2_iommu_dump_ctx() warn: expected subtract in snprintf limit 'maxcol'
drivers/iommu/omap-iommu-debug.c:62 omap2_iommu_dump_ctx() warn: expected subtract in snprintf limit 'maxcol'
drivers/iommu/omap-iommu-debug.c:63 omap2_iommu_dump_ctx() warn: expected subtract in snprintf limit 'maxcol'
drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm_debugfs.c:217 dp_link_settings_read() warn: expected subtract in snprintf limit 'str_len'
drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm_debugfs.c:224 dp_link_settings_read() warn: expected subtract in snprintf limit 'str_len'
drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm_debugfs.c:231 dp_link_settings_read() warn: expected subtract in snprintf limit 'str_len'
drivers/gpu/drm/drm_print.c:144 __drm_printfn_coredump() warn: expected subtract in snprintf limit 'iterator->remain'
drivers/md/dm-ima.c:337 dm_ima_measure_on_table_load() warn: expected subtract in snprintf limit '3'
drivers/scsi/fcoe/fcoe_transport.c:608 fcoe_transport_show() warn: expected subtract in snprintf limit '16'
drivers/scsi/fcoe/fcoe_transport.c:612 fcoe_transport_show() warn: expected subtract in snprintf limit '16'
drivers/acpi/device_sysfs.c:228 create_of_modalias() warn: expected subtract in snprintf limit 'size'
drivers/input/misc/keyspan_remote.c:132 keyspan_print() warn: expected subtract in snprintf limit '4'
drivers/accel/habanalabs/gaudi/gaudi.c:8927 gaudi_fill_sobs_from_mon() warn: expected subtract in snprintf limit 'max_write'
drivers/accel/habanalabs/gaudi/gaudi.c:8930 gaudi_fill_sobs_from_mon() warn: expected subtract in snprintf limit 'max_write'
drivers/hwmon/pmbus/ibm-cffps.c:169 ibm_cffps_debugfs_read() warn: expected subtract in snprintf limit '3'
drivers/hwmon/pmbus/ibm-cffps.c:180 ibm_cffps_debugfs_read() warn: expected subtract in snprintf limit '5'
fs/proc/bootconfig.c:53 copy_xbc_key_value_list() warn: expected subtract in snprintf limit '(end > dst) ?end - dst:0'
fs/proc/bootconfig.c:60 copy_xbc_key_value_list() warn: expected subtract in snprintf limit '(end > dst) ?end - dst:0'
fs/proc/bootconfig.c:67 copy_xbc_key_value_list() warn: expected subtract in snprintf limit '(end > dst) ?end - dst:0'
fs/ecryptfs/crypto.c:127 ecryptfs_derive_iv() warn: expected subtract in snprintf limit '16'
sound/soc/intel/avs/topology.c:191 avs_parse_string_token() warn: expected subtract in snprintf limit '44'

[PATCH 3/4] ACPI: sysfs: Remove some useless trailing NULL writes

[Christophe JAILLET]

'modalias' is only written with snprintf(), so it is already guaranteed to
be NULL terminated.

Remove the unneeded (but harmless) writes of a trailing '\0'.

Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
---
 drivers/acpi/device_sysfs.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/acpi/device_sysfs.c b/drivers/acpi/device_sysfs.c
index 7ec3142f3eda..1cf6568a813f 100644
--- a/drivers/acpi/device_sysfs.c
+++ b/drivers/acpi/device_sysfs.c
@@ -177,7 +177,7 @@ static int create_pnp_modalias(const struct acpi_device *acpi_dev, char *modalia
 		len += count;
 		size -= count;
 	}
-	modalias[len] = '\0';
+
 	return len;
 }
 
@@ -237,7 +237,7 @@ static int create_of_modalias(const struct acpi_device *acpi_dev, char *modalias
 		len += count;
 		size -= count;
 	}
-	modalias[len] = '\0';
+
 	return len;
 }
 
-- 
2.32.0

[PATCH 4/4] ACPI: sysfs: Remove some dead code

[Christophe JAILLET]

snprintf() never returns <0 values.
So remove some dead code.

Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
---
 drivers/acpi/device_sysfs.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/acpi/device_sysfs.c b/drivers/acpi/device_sysfs.c
index 1cf6568a813f..23373faa35ec 100644
--- a/drivers/acpi/device_sysfs.c
+++ b/drivers/acpi/device_sysfs.c
@@ -168,8 +168,6 @@ static int create_pnp_modalias(const struct acpi_device *acpi_dev, char *modalia
 			continue;
 
 		count = snprintf(&modalias[len], size, "%s:", id->id);
-		if (count < 0)
-			return -EINVAL;
 
 		if (count >= size)
 			return -ENOMEM;
@@ -228,8 +226,6 @@ static int create_of_modalias(const struct acpi_device *acpi_dev, char *modalias
 	for (i = 0; i < nval; i++, obj++) {
 		count = snprintf(&modalias[len], size, "C%s",
 				 obj->string.pointer);
-		if (count < 0)
-			return -EINVAL;
 
 		if (count >= size)
 			return -ENOMEM;
-- 
2.32.0

Re: [PATCH 0/4] ACPI: sysfs: Fix some issues in create_of_modalias() and create_pnp_modalias()

[Rafael J. Wysocki]

On Mon, Oct 23, 2023 at 7:32 PM Christophe JAILLET
<christophe.jaillet@wanadoo.fr> wrote:
>
> All issues have been introduced by the same commit, 8765c5ba1949 ("ACPI
> / scan: Rework modalias creation when "compatible" is present")
>
> The first 2 patches fixe some issues related to string truncation checks
> and to computation of the available space in the output buffer.
>
> The 2 others are just some clean-ups.
>
> Christophe JAILLET (4):
>   ACPI: sysfs: Fix the check for a potential string truncation
>   ACPI: sysfs: Fix a potential out-of-bound write in
>     create_of_modalias()
>   ACPI: sysfs: Remove some useless trailing NULL writes
>   ACPI: sysfs: Remove some dead code
>
>  drivers/acpi/device_sysfs.c | 18 ++++++++----------
>  1 file changed, 8 insertions(+), 10 deletions(-)
>
> --

Thanks for the fixes!

I would combine patch [1/4] with patch [2/4] and patch [3/4] with
patch [4/4], though.

If that's OK, I can do that while applying the patches.

Re: [PATCH 0/4] ACPI: sysfs: Fix some issues in create_of_modalias() and create_pnp_modalias()

[Christophe JAILLET]

Le 23/10/2023 à 22:09, Rafael J. Wysocki a écrit :
> On Mon, Oct 23, 2023 at 7:32 PM Christophe JAILLET
> <christophe.jaillet@wanadoo.fr> wrote:
>>
>> All issues have been introduced by the same commit, 8765c5ba1949 ("ACPI
>> / scan: Rework modalias creation when "compatible" is present")
>>
>> The first 2 patches fixe some issues related to string truncation checks
>> and to computation of the available space in the output buffer.
>>
>> The 2 others are just some clean-ups.
>>
>> Christophe JAILLET (4):
>>    ACPI: sysfs: Fix the check for a potential string truncation
>>    ACPI: sysfs: Fix a potential out-of-bound write in
>>      create_of_modalias()
>>    ACPI: sysfs: Remove some useless trailing NULL writes
>>    ACPI: sysfs: Remove some dead code
>>
>>   drivers/acpi/device_sysfs.c | 18 ++++++++----------
>>   1 file changed, 8 insertions(+), 10 deletions(-)
>>
>> --
> 
> Thanks for the fixes!
> 
> I would combine patch [1/4] with patch [2/4] and patch [3/4] with
> patch [4/4], though.
> 
> If that's OK, I can do that while applying the patches.
> 

Hi,

up to you.
Either way is fine for me.

CJ

Re: [PATCH 0/4] ACPI: sysfs: Fix some issues in create_of_modalias() and create_pnp_modalias()

[Christophe JAILLET]

Le 23/10/2023 à 22:09, Rafael J. Wysocki a écrit :
> On Mon, Oct 23, 2023 at 7:32 PM Christophe JAILLET
> <christophe.jaillet@wanadoo.fr> wrote:
>>
>> All issues have been introduced by the same commit, 8765c5ba1949 ("ACPI
>> / scan: Rework modalias creation when "compatible" is present")
>>
>> The first 2 patches fixe some issues related to string truncation checks
>> and to computation of the available space in the output buffer.
>>
>> The 2 others are just some clean-ups.
>>
>> Christophe JAILLET (4):
>>    ACPI: sysfs: Fix the check for a potential string truncation
>>    ACPI: sysfs: Fix a potential out-of-bound write in
>>      create_of_modalias()
>>    ACPI: sysfs: Remove some useless trailing NULL writes
>>    ACPI: sysfs: Remove some dead code
>>
>>   drivers/acpi/device_sysfs.c | 18 ++++++++----------
>>   1 file changed, 8 insertions(+), 10 deletions(-)
>>
>> --
> 
> Thanks for the fixes!
> 
> I would combine patch [1/4] with patch [2/4] and patch [3/4] with
> patch [4/4], though.
> 
> If that's OK, I can do that while applying the patches.
> 

Hi,

up to you.
Either way is fine for me.

CJ