Skip to content

Conversation

@modules-kpd-app
Copy link

Pull request for series with
subject: module: Fix memory deallocation on error path in move_module()
version: 2
url: https://patchwork.kernel.org/project/linux-modules/list/?series=973425

dagomez137 and others added 3 commits June 25, 2025 13:39
The function move_module() uses the variable t to track how many memory
types it has allocated and consequently how many should be freed if an
error occurs.

The variable is initially set to 0 and is updated when a call to
module_memory_alloc() fails. However, move_module() can fail for other
reasons as well, in which case t remains set to 0 and no memory is freed.

Fix the problem by initializing t to MOD_MEM_NUM_TYPES. Additionally, make
the deallocation loop more robust by not relying on the mod_mem_type_t enum
having a signed integer as its underlying type.

Fixes: c7ee8ae ("module: add stop-grap sanity check on module memcpy()")
Signed-off-by: Petr Pavlu <petr.pavlu@suse.com>
Reviewed-by: Sami Tolvanen <samitolvanen@google.com>
Reviewed-by: Daniel Gomez <da.gomez@samsung.com>
All error conditions in move_module() set the return value by updating the
ret variable. Therefore, it is not necessary to the initialize the variable
when declaring it.

Remove the unnecessary initialization.

Signed-off-by: Petr Pavlu <petr.pavlu@suse.com>
Reviewed-by: Sami Tolvanen <samitolvanen@google.com>
Reviewed-by: Daniel Gomez <da.gomez@samsung.com>
@modules-kpd-app
Copy link
Author

Upstream branch: a0b018a
series: https://patchwork.kernel.org/project/linux-modules/list/?series=973425
version: 2

@modules-kpd-app modules-kpd-app bot force-pushed the modules-next_base branch 2 times, most recently from 6381fa5 to aaa7ece Compare June 26, 2025 18:03
@dagomez137 dagomez137 force-pushed the modules-next_base branch 2 times, most recently from b4d1c4e to 61dc34e Compare July 4, 2025 19:48
@modules-kpd-app modules-kpd-app bot force-pushed the modules-next_base branch 2 times, most recently from 1df8185 to 9aab33d Compare July 8, 2025 19:00
@dagomez137 dagomez137 force-pushed the modules-next_base branch from 9aab33d to 99d099e Compare July 8, 2025 19:46
@modules-kpd-app modules-kpd-app bot force-pushed the modules-next_base branch from 99d099e to 3aed49d Compare July 8, 2025 19:56
@modules-kpd-app modules-kpd-app bot force-pushed the modules-next_base branch 3 times, most recently from 0299e02 to cdc6b9e Compare October 22, 2025 10:27
@modules-kpd-app
Copy link
Author

Upstream branch: 5eb4b9a
series: https://patchwork.kernel.org/project/linux-modules/list/?series=973425
version: 2

Pull request is NOT updated. Failed to apply https://patchwork.kernel.org/project/linux-modules/list/?series=973425
error message:

Cmd('git') failed due to: exit code(128)
  cmdline: git am --3way
  stdout: 'Applying: module: Fix memory deallocation on error path in move_module()
Using index info to reconstruct a base tree...
M	kernel/module/main.c
Falling back to patching base and 3-way merge...
Auto-merging kernel/module/main.c
CONFLICT (content): Merge conflict in kernel/module/main.c
Patch failed at 0001 module: Fix memory deallocation on error path in move_module()'
  stderr: 'error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
hint: When you have resolved this problem, run "git am --continue".
hint: If you prefer to skip this patch, run "git am --skip" instead.
hint: To restore the original branch and stop patching, run "git am --abort".
hint: Disable this message with "git config advice.mergeConflict false"'

conflict:

diff --cc kernel/module/main.c
index c66b26184936,9ac994b2f354..000000000000
--- a/kernel/module/main.c
+++ b/kernel/module/main.c
@@@ -2723,8 -2696,9 +2723,14 @@@ static int find_module_sections(struct 
  
  static int move_module(struct module *mod, struct load_info *info)
  {
++<<<<<<< HEAD
 +	int i, ret;
 +	enum mod_mem_type t = MOD_MEM_NUM_TYPES;
++=======
+ 	int i;
+ 	enum mod_mem_type t = MOD_MEM_NUM_TYPES;
+ 	int ret = -ENOMEM;
++>>>>>>> module: Fix memory deallocation on error path in move_module()
  	bool codetag_section_found = false;
  
  	for_each_mod_mem_type(type) {

dagomez137 pushed a commit that referenced this pull request Jan 26, 2026
Revert commit bfc467d ("serial: remove redundant
tty_port_link_device()") because the tty_port_link_device() is not
redundant: the tty->port has to be confured before we call
uart_configure_port(), otherwise user-space can open console without TTY
linked to the driver.

This tty_port_link_device() was added explicitly to avoid this exact
issue in commit fb2b900 ("tty: link tty and port before configuring
it as console"), so offending commit basically reverted the fix saying
it is redundant without addressing the actual race condition presented
there.

Reproducible always as tty->port warning on Qualcomm SoC with most of
devices disabled, so with very fast boot, and one serial device being
the console:

  printk: legacy console [ttyMSM0] enabled
  printk: legacy console [ttyMSM0] enabled
  printk: legacy bootconsole [qcom_geni0] disabled
  printk: legacy bootconsole [qcom_geni0] disabled
  ------------[ cut here ]------------
  tty_init_dev: ttyMSM driver does not set tty->port. This would crash the kernel. Fix the driver!
  WARNING: drivers/tty/tty_io.c:1414 at tty_init_dev.part.0+0x228/0x25c, CPU#2: systemd/1
  Modules linked in: socinfo tcsrcc_eliza gcc_eliza sm3_ce fuse ipv6
  CPU: 2 UID: 0 PID: 1 Comm: systemd Tainted: G S                  6.19.0-rc4-next-20260108-00024-g2202f4d30aa8 #73 PREEMPT
  Tainted: [S]=CPU_OUT_OF_SPEC
  Hardware name: Qualcomm Technologies, Inc. Eliza (DT)
  ...
  tty_init_dev.part.0 (drivers/tty/tty_io.c:1414 (discriminator 11)) (P)
  tty_open (arch/arm64/include/asm/atomic_ll_sc.h:95 (discriminator 3) drivers/tty/tty_io.c:2073 (discriminator 3) drivers/tty/tty_io.c:2120 (discriminator 3))
  chrdev_open (fs/char_dev.c:411)
  do_dentry_open (fs/open.c:962)
  vfs_open (fs/open.c:1094)
  do_open (fs/namei.c:4634)
  path_openat (fs/namei.c:4793)
  do_filp_open (fs/namei.c:4820)
  do_sys_openat2 (fs/open.c:1391 (discriminator 3))
  ...
  Starting Network Name Resolution...

Apparently the flow with this small Yocto-based ramdisk user-space is:

driver (qcom_geni_serial.c):                  user-space:
============================                  ===========
qcom_geni_serial_probe()
 uart_add_one_port()
  serial_core_register_port()
   serial_core_add_one_port()
    uart_configure_port()
     register_console()
    |
    |                                         open console
    |                                          ...
    |                                          tty_init_dev()
    |                                           driver->ports[idx] is NULL
    |
    tty_port_register_device_attr_serdev()
     tty_port_link_device() <- set driver->ports[idx]

Fixes: bfc467d ("serial: remove redundant tty_port_link_device()")
Cc: stable@vger.kernel.org
Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
Reviewed-by: Jiri Slaby <jirislaby@kernel.org>
Link: https://patch.msgid.link/20260123072139.53293-2-krzysztof.kozlowski@oss.qualcomm.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants