All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sasha Levin <sashal@kernel.org>
To: patches@lists.linux.dev, stable@vger.kernel.org
Cc: Petr Pavlu <petr.pavlu@suse.com>,
	Daniel Gomez <da.gomez@samsung.com>,
	Sasha Levin <sashal@kernel.org>,
	mcgrof@kernel.org, da.gomez@kernel.org,
	linux-modules@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: [PATCH AUTOSEL 6.16-6.1] module: Prevent silent truncation of module name in delete_module(2)
Date: Fri,  8 Aug 2025 11:30:49 -0400	[thread overview]
Message-ID: <20250808153054.1250675-9-sashal@kernel.org> (raw)
In-Reply-To: <20250808153054.1250675-1-sashal@kernel.org>

From: Petr Pavlu <petr.pavlu@suse.com>

[ Upstream commit a6323bd4e611567913e23df5b58f2d4e4da06789 ]

Passing a module name longer than MODULE_NAME_LEN to the delete_module
syscall results in its silent truncation. This really isn't much of
a problem in practice, but it could theoretically lead to the removal of an
incorrect module. It is more sensible to return ENAMETOOLONG or ENOENT in
such a case.

Update the syscall to return ENOENT, as documented in the delete_module(2)
man page to mean "No module by that name exists." This is appropriate
because a module with a name longer than MODULE_NAME_LEN cannot be loaded
in the first place.

Signed-off-by: Petr Pavlu <petr.pavlu@suse.com>
Reviewed-by: Daniel Gomez <da.gomez@samsung.com>
Link: https://lore.kernel.org/r/20250630143535.267745-2-petr.pavlu@suse.com
Signed-off-by: Daniel Gomez <da.gomez@samsung.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---

LLM Generated explanations, may be completely bogus:

Based on my analysis, here's my assessment:

**Backport Status: YES**

This commit should be backported to stable kernel trees for the
following reasons:

1. **Security Fix for Silent Truncation**: The commit fixes a security-
   relevant issue where module names longer than MODULE_NAME_LEN (56
   bytes on 64-bit systems) are silently truncated in the
   delete_module() syscall. This could theoretically lead to
   unintentionally removing the wrong module if two modules share the
   same truncated prefix.

2. **Minimal and Contained Change**: The fix is very small and localized
   - it only modifies the delete_module syscall implementation in
   kernel/module/main.c. The changes are:
   - Changes from `strncpy_from_user(name, name_user,
     MODULE_NAME_LEN-1)` to `strncpy_from_user(name, name_user,
     MODULE_NAME_LEN)`
   - Adds proper length checking: `if (len == 0 || len ==
     MODULE_NAME_LEN) return -ENOENT;`
   - Returns the correct error code (ENOENT) for oversized names

3. **Follows Stable Tree Rules**:
   - Fixes a bug that could affect users (incorrect module removal)
   - Very low risk of regression - the change only affects error
     handling for invalid input
   - No new features or architectural changes
   - Improves consistency with documented behavior (delete_module(2) man
     page)

4. **Prevents Inconsistent Behavior**: The commit message correctly
   points out that modules with names longer than MODULE_NAME_LEN cannot
   be loaded in the first place, so returning ENOENT for such names in
   delete_module makes the behavior consistent across module operations.

5. **Clear Bug Fix**: The old code would accept a 57+ character module
   name, truncate it to 56 characters, and potentially remove a
   different module. The new code properly rejects such names with
   ENOENT, preventing this dangerous silent truncation.

The change is defensive programming that prevents a potential security
issue without introducing any backward compatibility concerns, making it
an ideal candidate for stable backporting.

 kernel/module/main.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/kernel/module/main.c b/kernel/module/main.c
index c2c08007029d..cbd637627eb4 100644
--- a/kernel/module/main.c
+++ b/kernel/module/main.c
@@ -751,14 +751,16 @@ SYSCALL_DEFINE2(delete_module, const char __user *, name_user,
 	struct module *mod;
 	char name[MODULE_NAME_LEN];
 	char buf[MODULE_FLAGS_BUF_SIZE];
-	int ret, forced = 0;
+	int ret, len, forced = 0;
 
 	if (!capable(CAP_SYS_MODULE) || modules_disabled)
 		return -EPERM;
 
-	if (strncpy_from_user(name, name_user, MODULE_NAME_LEN-1) < 0)
-		return -EFAULT;
-	name[MODULE_NAME_LEN-1] = '\0';
+	len = strncpy_from_user(name, name_user, MODULE_NAME_LEN);
+	if (len == 0 || len == MODULE_NAME_LEN)
+		return -ENOENT;
+	if (len < 0)
+		return len;
 
 	audit_log_kern_module(name);
 
-- 
2.39.5


  parent reply	other threads:[~2025-08-08 15:31 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-08-08 15:30 [PATCH AUTOSEL 6.16-6.6] apparmor: shift ouid when mediating hard links in userns Sasha Levin
2025-08-08 15:30 ` [PATCH AUTOSEL 6.16-5.10] md: dm-zoned-target: Initialize return variable r to avoid uninitialized use Sasha Levin
2025-08-08 15:30 ` [PATCH AUTOSEL 6.16-5.4] i3c: don't fail if GETHDRCAP is unsupported Sasha Levin
2025-08-08 15:30   ` Sasha Levin
2025-08-08 15:30 ` [PATCH AUTOSEL 6.16-5.10] dm-mpath: don't print the "loaded" message if registering fails Sasha Levin
2025-08-08 15:30 ` [PATCH AUTOSEL 6.16-5.10] rtc: ds1307: handle oscillator stop flag (OSF) for ds1341 Sasha Levin
2025-08-11 16:46   ` Meagan Lloyd
2025-08-16 13:07     ` Sasha Levin
2025-08-08 15:30 ` [PATCH AUTOSEL 6.16-5.4] i3c: add missing include to internal header Sasha Levin
2025-08-08 15:30   ` Sasha Levin
2025-08-08 15:30 ` [PATCH AUTOSEL 6.16-6.1] i3c: master: Initialize ret in i3c_i2c_notifier_call() Sasha Levin
2025-08-08 15:30   ` Sasha Levin
2025-08-08 15:30 ` [PATCH AUTOSEL 6.16-5.4] PCI: pnv_php: Work around switches with broken presence detection Sasha Levin
2025-08-08 15:30 ` Sasha Levin [this message]
2025-08-08 15:30 ` [PATCH AUTOSEL 6.16-6.1] apparmor: use the condition in AA_BUG_FMT even with debug disabled Sasha Levin
2025-08-08 15:30 ` [PATCH AUTOSEL 6.16-6.6] powerpc/eeh: Make EEH driver device hotplug safe Sasha Levin
2025-08-08 15:30 ` [PATCH AUTOSEL 6.16-6.12] apparmor: fix x_table_lookup when stacking is not the first entry Sasha Levin
2025-08-08 15:30 ` [PATCH AUTOSEL 6.16-6.1] dm-table: fix checking for rq stackable devices Sasha Levin
2025-08-08 15:30 ` [PATCH AUTOSEL 6.16-5.10] PCI: pnv_php: Clean up allocated IRQs on unplug Sasha Levin
2025-08-08 15:59   ` Timothy Pearson
2025-08-08 17:04     ` Sasha Levin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20250808153054.1250675-9-sashal@kernel.org \
    --to=sashal@kernel.org \
    --cc=da.gomez@kernel.org \
    --cc=da.gomez@samsung.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-modules@vger.kernel.org \
    --cc=mcgrof@kernel.org \
    --cc=patches@lists.linux.dev \
    --cc=petr.pavlu@suse.com \
    --cc=stable@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.