* Re: [PATCH v2 2/4] module: show why force load fails
From: Petr Pavlu @ 2025-08-26 9:33 UTC (permalink / raw)
To: Jinchao Wang
Cc: Luis Chamberlain, Daniel Gomez, Sami Tolvanen, linux-modules,
linux-kernel
In-Reply-To: <20250825091545.18607-3-wangjinchao600@gmail.com>
On 8/25/25 11:15 AM, Jinchao Wang wrote:
> Include reason in error message when force loading is disabled.
>
> Signed-off-by: Jinchao Wang <wangjinchao600@gmail.com>
> ---
> kernel/module/main.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/kernel/module/main.c b/kernel/module/main.c
> index c66b26184936..a426bd8a18b5 100644
> --- a/kernel/module/main.c
> +++ b/kernel/module/main.c
> @@ -1083,6 +1083,7 @@ int try_to_force_load(struct module *mod, const char *reason)
> add_taint_module(mod, TAINT_FORCED_MODULE, LOCKDEP_NOW_UNRELIABLE);
> return 0;
> #else
> + pr_err("%s force load is not supported\n", reason);
> return -ENOEXEC;
> #endif
> }
The module name is already available at all points where
try_to_force_load() is called, so the new error message should include
it.
Additionally, we should be careful about the message. In the case of the
init_module syscall, the missing modversions and vermagic could mean
that the data was deliberately stripped by kmod because the module was
inserted with --force, or it could mean that the module lacks this data
in the first place. In other words, it is not always the case that that
we're reaching this logic because of a force load.
My suggestion would be to use the following:
pr_err("%s: %s, force load is not supported\n", mod->name, reason);
--
Thanks,
Petr
^ permalink raw reply
* Re: [PATCH] module: pr_debug when there is no version info
From: Petr Pavlu @ 2025-08-26 7:20 UTC (permalink / raw)
To: Wang Jinchao
Cc: Luis Chamberlain, Daniel Gomez, Sami Tolvanen, linux-kernel,
linux-modules
In-Reply-To: <9b768f91-121a-4072-88b2-36cb48be3917@suse.com>
On 7/22/25 10:25 AM, Petr Pavlu wrote:
> On 7/22/25 5:08 AM, Wang Jinchao wrote:
>> On 7/21/25 22:40, Petr Pavlu wrote:
>>> On 7/21/25 6:52 AM, Wang Jinchao wrote:
>>>> When there is no version information, modprobe and insmod only
>>>> report "invalid format".
>>>> Print the actual cause to make it easier to diagnose the issue.
>>>> This helps developers quickly identify version-related module
>>>> loading failures.
>>>> Signed-off-by: Wang Jinchao <wangjinchao600@gmail.com>
>>>> ---
>>>> kernel/module/version.c | 4 +++-
>>>> 1 file changed, 3 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/kernel/module/version.c b/kernel/module/version.c
>>>> index 2beefeba82d9..bc28c697ff3a 100644
>>>> --- a/kernel/module/version.c
>>>> +++ b/kernel/module/version.c
>>>> @@ -42,8 +42,10 @@ int check_version(const struct load_info *info,
>>>> }
>>>> /* No versions at all? modprobe --force does this. */
>>>> - if (versindex == 0)
>>>> + if (versindex == 0) {
>>>> + pr_debug("No version info for module %s\n", info->name);
>>>> return try_to_force_load(mod, symname) == 0;
>>>> + }
>>>> versions = (void *)sechdrs[versindex].sh_addr;
>>>> num_versions = sechdrs[versindex].sh_size
>>>
>>> I think it would be better to instead improve the behavior of
>>> try_to_force_load(). The function should print the error reason prior to
>>> returning -ENOEXEC. This would also help its two other callers,
>>> check_modinfo() and check_export_symbol_versions().
>>>
>>> Additionally, I suggest moving the check to ensure version information
>>> is available for imported symbols earlier in the loading process.
>>> A suitable place might be check_modstruct_version(). This way the check
>>> is performed only once per module.
>>>
>>> The following is a prototype patch:
>>>
>>> diff --git a/kernel/module/main.c b/kernel/module/main.c
>>> index c2c08007029d..c1ccd343e8c3 100644
>>> --- a/kernel/module/main.c
>>> +++ b/kernel/module/main.c
>>> @@ -1053,6 +1053,7 @@ int try_to_force_load(struct module *mod, const char *reason)
>>> add_taint_module(mod, TAINT_FORCED_MODULE, LOCKDEP_NOW_UNRELIABLE);
>>> return 0;
>>> #else
>>> + pr_err("%s: %s\n", mod->name, reason);
>>> return -ENOEXEC;
>>> #endif
>>> }
>>> diff --git a/kernel/module/version.c b/kernel/module/version.c
>>> index 2beefeba82d9..4d9ebf0834de 100644
>>> --- a/kernel/module/version.c
>>> +++ b/kernel/module/version.c
>>> @@ -41,9 +41,9 @@ int check_version(const struct load_info *info,
>>> return 1;
>>> }
>>> - /* No versions at all? modprobe --force does this. */
>>> + /* No versions? Ok, already tainted in check_modstruct_version(). */
>>> if (versindex == 0)
>>> - return try_to_force_load(mod, symname) == 0;
>>> + return 1;
>>> versions = (void *)sechdrs[versindex].sh_addr;
>>> num_versions = sechdrs[versindex].sh_size
>>> @@ -90,6 +90,11 @@ int check_modstruct_version(const struct load_info *info,
>>> have_symbol = find_symbol(&fsa);
>>> BUG_ON(!have_symbol);
>>> + /* No versions at all? modprobe --force does this. */
>>> + if (!info->index.vers && !info->index.vers_ext_crc)
>>> + return try_to_force_load(
>>> + mod, "no versions for imported symbols") == 0;
>>> +
>>> return check_version(info, "module_layout", mod, fsa.crc);
>>> }
>>>
>>> As a side note, I'm confused why with CONFIG_MODULE_FORCE_LOAD=y, the
>>> code treats missing modversions for imported symbols as ok, even without
>>> MODULE_INIT_IGNORE_MODVERSIONS. This is at least consistent with the
>>> handling of missing vermagic, but it seems this behavior should be
>>> stricter.
>>>
>> When debugging syzkaller, I noticed that the insmod command always reports errors. However, to get the exact information, I need to trace the kernel, so I casually submitted this patch.
>>
>> Based on your response, I also feel that the meaning of force_load here is somewhat unclear. It would be better to create a mask or a clear list to indicate which fields can be forced and which cannot. Once this is clear, we can create a function named may_force_check().
>
> I cannot find an explicit reason in the Git history why a missing
> vermagic is treated as if the module was loaded with
> MODULE_INIT_IGNORE_VERMAGIC, and similarly why missing modversions data
> is treated as if the module was loaded with
> MODULE_INIT_IGNORE_MODVERSIONS.
>
> I would argue that a more sensible behavior would be to consider
> a missing vermagic as an error and allow loading the module only if
> MODULE_INIT_IGNORE_VERMAGIC is explicitly specified. And analogously for
> missing modversions and MODULE_INIT_IGNORE_MODVERSIONS.
>
> Nonetheless, if I understand correctly, this should be mostly separate
> from your issue.
To answer my own confusion, the thing that I missed is that the
MODULE_INIT_IGNORE_* flags are available only for the finit_module
syscall, not for init_module. In the case of init_module, the force
logic is handled by kmod in userspace by stripping the relevant
modversions and vermagic data. This means that when using init_module,
the module loader cannot distinguish between a module that lacks this
data and one that has it deliberately removed. When finit_module was
introduced in 2012, commit 2f3238aebedb ("module: add flags arg to
sys_finit_module()") added the MODULE_INIT_IGNORE_* flags, and they were
simply implemented to mirror the kmod behavior.
-- Petr
^ permalink raw reply
* Re: [PATCH v6 4/9] scsi: Always define blogic_pci_tbl structure
From: Nathan Chancellor @ 2025-08-25 16:56 UTC (permalink / raw)
To: Alexey Gladkov, Linus Walleij
Cc: Martin K. Petersen, Nicolas Schier, Masahiro Yamada, Petr Pavlu,
Luis Chamberlain, Sami Tolvanen, Daniel Gomez, linux-kernel,
linux-modules, linux-kbuild, Khalid Aziz, linux-scsi,
James Bottomley, Arnd Bergmann, Damien Le Moal
In-Reply-To: <aKw2bKTjmHBGjxt_@example.org>
Hi Alexey,
On Mon, Aug 25, 2025 at 12:09:48PM +0200, Alexey Gladkov wrote:
> Has these patches been added somewhere, I can't find it in kbuild?
I plan to apply this to the kbuild tree when [1] becomes available in
mainline so that I can backmerge it and apply the rest of the series on
top to avoid build breakage. As it is on a fixes branch, I would expect
it to be there by -rc4 or -rc5, which still gives us a few weeks for
soak testing in -next. If it is not there by then, I will just apply the
series in whole and we will just end up with the same commit in the tree
twice.
[1]: https://git.kernel.org/linusw/linux-pinctrl/c/bd7c2312128e31d056d30d34d60503de056e15f0
Cheers,
Nathan
^ permalink raw reply
* Re: [PATCH v6 4/9] scsi: Always define blogic_pci_tbl structure
From: Alexey Gladkov @ 2025-08-25 10:09 UTC (permalink / raw)
To: Nathan Chancellor
Cc: Martin K. Petersen, Nicolas Schier, Masahiro Yamada, Petr Pavlu,
Luis Chamberlain, Sami Tolvanen, Daniel Gomez, linux-kernel,
linux-modules, linux-kbuild, Khalid Aziz, linux-scsi,
James Bottomley, Arnd Bergmann, Damien Le Moal
In-Reply-To: <20250820161142.GB3805667@ax162>
On Wed, Aug 20, 2025 at 09:11:42AM -0700, Nathan Chancellor wrote:
> On Tue, Aug 19, 2025 at 09:52:10PM -0400, Martin K. Petersen wrote:
> > >> Applied to 6.18/scsi-staging, thanks!
> > >
> > > I think I will need this change to apply patch 7 [1] to kbuild-next
> > > without any issues [2]. If there is little risk of conflict, could I
> > > take it with your Ack?
> >
> > Sure, no problem. Dropped the patch from my tree.
> >
> > Acked-by: Martin K. Petersen <martin.petersen@oracle.com>
>
> Thanks a lot!
Has these patches been added somewhere, I can't find it in kbuild?
--
Rgrds, legion
^ permalink raw reply
* [PATCH v2 4/4] module: separate vermagic and livepatch checks
From: Jinchao Wang @ 2025-08-25 9:15 UTC (permalink / raw)
To: Luis Chamberlain, Petr Pavlu, Daniel Gomez, Sami Tolvanen,
linux-modules, linux-kernel
Cc: Jinchao Wang
In-Reply-To: <20250825091545.18607-1-wangjinchao600@gmail.com>
Rename check_modinfo() to check_modinfo_vermagic() to clarify it only
checks vermagic compatibility.
Move livepatch check to happen after vermagic check in early_mod_check(),
creating proper separation of concerns.
This makes the module loading sequence more logical:
- First verify module vermagic
- Then check livepatch-specific requirements
No functional changes, just clearer code organization.
Signed-off-by: Jinchao Wang <wangjinchao600@gmail.com>
---
kernel/module/main.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/kernel/module/main.c b/kernel/module/main.c
index a426bd8a18b5..d30bffeef63e 100644
--- a/kernel/module/main.c
+++ b/kernel/module/main.c
@@ -2571,7 +2571,7 @@ static void module_augment_kernel_taints(struct module *mod, struct load_info *i
}
-static int check_modinfo(struct module *mod, struct load_info *info, int flags)
+static int check_modinfo_vermagic(struct module *mod, struct load_info *info, int flags)
{
const char *modmagic = get_modinfo(info, "vermagic");
int err;
@@ -2590,10 +2590,6 @@ static int check_modinfo(struct module *mod, struct load_info *info, int flags)
return -ENOEXEC;
}
- err = check_modinfo_livepatch(mod, info);
- if (err)
- return err;
-
return 0;
}
@@ -3334,7 +3330,11 @@ static int early_mod_check(struct load_info *info, int flags)
if (!check_modstruct_version(info, info->mod))
return -ENOEXEC;
- err = check_modinfo(info->mod, info, flags);
+ err = check_modinfo_vermagic(info->mod, info, flags);
+ if (err)
+ return err;
+
+ err = check_modinfo_livepatch(info->mod, info);
if (err)
return err;
--
2.43.0
^ permalink raw reply related
* [PATCH v2 3/4] module: centralize no-versions force load check
From: Jinchao Wang @ 2025-08-25 9:15 UTC (permalink / raw)
To: Luis Chamberlain, Petr Pavlu, Daniel Gomez, Sami Tolvanen,
linux-modules, linux-kernel
Cc: Jinchao Wang
In-Reply-To: <20250825091545.18607-1-wangjinchao600@gmail.com>
Move try_to_force_load() call from check_version() to
check_modstruct_version() to handle "no versions" case only once before
other version checks.
This prevents duplicate force load attempts and makes the error message
show the proper reason.
Signed-off-by: Jinchao Wang <wangjinchao600@gmail.com>
---
kernel/module/version.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/kernel/module/version.c b/kernel/module/version.c
index 2beefeba82d9..3f07fd03cb30 100644
--- a/kernel/module/version.c
+++ b/kernel/module/version.c
@@ -41,10 +41,6 @@ int check_version(const struct load_info *info,
return 1;
}
- /* No versions at all? modprobe --force does this. */
- if (versindex == 0)
- return try_to_force_load(mod, symname) == 0;
-
versions = (void *)sechdrs[versindex].sh_addr;
num_versions = sechdrs[versindex].sh_size
/ sizeof(struct modversion_info);
@@ -81,6 +77,11 @@ int check_modstruct_version(const struct load_info *info,
};
bool have_symbol;
+ /* No versions at all? modprobe --force does this. */
+ if (info->index.vers == 0 &&
+ try_to_force_load(mod, "no versions module"))
+ return 1;
+
/*
* Since this should be found in kernel (which can't be removed), no
* locking is necessary. Regardless use a RCU read section to keep
--
2.43.0
^ permalink raw reply related
* [PATCH v2 2/4] module: show why force load fails
From: Jinchao Wang @ 2025-08-25 9:15 UTC (permalink / raw)
To: Luis Chamberlain, Petr Pavlu, Daniel Gomez, Sami Tolvanen,
linux-modules, linux-kernel
Cc: Jinchao Wang
In-Reply-To: <20250825091545.18607-1-wangjinchao600@gmail.com>
Include reason in error message when force loading is disabled.
Signed-off-by: Jinchao Wang <wangjinchao600@gmail.com>
---
kernel/module/main.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/kernel/module/main.c b/kernel/module/main.c
index c66b26184936..a426bd8a18b5 100644
--- a/kernel/module/main.c
+++ b/kernel/module/main.c
@@ -1083,6 +1083,7 @@ int try_to_force_load(struct module *mod, const char *reason)
add_taint_module(mod, TAINT_FORCED_MODULE, LOCKDEP_NOW_UNRELIABLE);
return 0;
#else
+ pr_err("%s force load is not supported\n", reason);
return -ENOEXEC;
#endif
}
--
2.43.0
^ permalink raw reply related
* [PATCH v2 1/4] module: signing: Use pr_err for signature rejection
From: Jinchao Wang @ 2025-08-25 9:15 UTC (permalink / raw)
To: Luis Chamberlain, Petr Pavlu, Daniel Gomez, Sami Tolvanen,
linux-modules, linux-kernel
Cc: Jinchao Wang
In-Reply-To: <20250825091545.18607-1-wangjinchao600@gmail.com>
Make module signature rejection messages more visible by using pr_err
instead of pr_notice.
Signed-off-by: Jinchao Wang <wangjinchao600@gmail.com>
---
kernel/module/signing.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/kernel/module/signing.c b/kernel/module/signing.c
index a2ff4242e623..557cb795fa31 100644
--- a/kernel/module/signing.c
+++ b/kernel/module/signing.c
@@ -117,7 +117,7 @@ int module_sig_check(struct load_info *info, int flags)
}
if (is_module_sig_enforced()) {
- pr_notice("Loading of %s is rejected\n", reason);
+ pr_err("Loading of %s is rejected\n", reason);
return -EKEYREJECTED;
}
--
2.43.0
^ permalink raw reply related
* [PATCH v2 0/4] module: logging and code improvements
From: Jinchao Wang @ 2025-08-25 9:15 UTC (permalink / raw)
To: Luis Chamberlain, Petr Pavlu, Daniel Gomez, Sami Tolvanen,
linux-modules, linux-kernel
Cc: Jinchao Wang
This series of patches cleans up and refactors the kernel's module loading
code. The goal is to make the subsystem's logging clearer and its internal
logic more straightforward for developers to understand.
The patches in this series:
- module: signing: Use pr_err for signature rejection
Makes module signature rejection messages more visible.
- module: show why force load fails
Adds a reason to the error message when force loading is disabled.
- module: centralize no-versions force load check
Refactors the code to centralize the "no versions" force load check.
- module: separate vermagic and livepatch checks
Improves code organization by separating vermagic and livepatch checks.
Changes from v1:
A patch was dropped because it was based on a misunderstanding
of the ignore versioning flag's original intent.
v1 link:
https://lore.kernel.org/all/20250822125454.1287066-1-wangjinchao600@gmail.com/T/#mf748b6e97934f7a463dfdafbb426965f3e0ad646
Jinchao Wang (4):
module: signing: Use pr_err for signature rejection
module: show why force load fails
module: centralize no-versions force load check
module: separate vermagic and livepatch checks
kernel/module/main.c | 13 +++++++------
kernel/module/signing.c | 2 +-
kernel/module/version.c | 9 +++++----
3 files changed, 13 insertions(+), 11 deletions(-)
--
2.43.0
^ permalink raw reply
* Re: [PATCH 1/5] module: Fix module_sig_check() for modules with ignored modversions/vermagic
From: Jinchao Wang @ 2025-08-25 3:17 UTC (permalink / raw)
To: Sami Tolvanen
Cc: Luis Chamberlain, Petr Pavlu, Daniel Gomez, linux-modules,
linux-kernel
In-Reply-To: <CABCJKucGtbZw_DCpnbUr7cQeU+_DF97YTeDVgPX7tTyPwNabog@mail.gmail.com>
On 8/23/25 03:36, Sami Tolvanen wrote:
> On Fri, Aug 22, 2025 at 5:55 AM Jinchao Wang <wangjinchao600@gmail.com> wrote:
>>
>> The current signature check logic incorrectly fails modules that have
>> valid signatures when the caller specifies MODULE_INIT_IGNORE_MODVERSIONS
>> or MODULE_INIT_IGNORE_VERMAGIC flags. This happens because the code
>> treats these flags as indicating a "mangled module" and skips signature
>> verification entirely.
>>
>> The key insight is that the intent of the caller (to ignore modversions
>> or vermagic) should not affect signature verification. A module with
>> a valid signature should be verified regardless of whether the caller
>> wants to ignore versioning information.
>
> Why would you need to ignore versions when loading signed modules?
> Here's the original series that added this check and I feel it's very
> much relevant still:
>
> https://lore.kernel.org/lkml/20160423184421.GL3348@decadent.org.uk/
>
> Sami
Hi Sami,
Thanks for explaining the historical context. I think there are two
possible understandings of "ignore."
The original seems to be "do not check, but still taint the module." My
patch was based on the understanding that "ignore" means to allow the
module, even if it is not signed or is signed with a different key.
Given your feedback, I've decided to drop the patch for now.
--
Best regards,
Jinchao
^ permalink raw reply
* Re: [GIT PULL] Modules fixes for v6.17-rc3
From: pr-tracker-bot @ 2025-08-24 14:19 UTC (permalink / raw)
To: Daniel Gomez
Cc: Linus Torvalds, Luis Chamberlain, Petr Pavlu, Sami Tolvanen,
Daniel Gomez, Thorsten Blum, Kees Cook, linux-modules,
linux-kernel
In-Reply-To: <20250824064922.41894-1-da.gomez@kernel.org>
The pull request you sent on Sun, 24 Aug 2025 08:49:17 +0200:
> git://git.kernel.org/pub/scm/linux/kernel/git/modules/linux.git tags/modules-6.17-rc3.fixes
has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/14f84cd318bed3fc64c6e4ee6b251f9b6a8e2a05
Thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/prtracker.html
^ permalink raw reply
* Re: [GIT PULL] Modules fixes for v6.17-rc3
From: Daniel Gomez @ 2025-08-24 7:00 UTC (permalink / raw)
To: Linus Torvalds
Cc: Luis Chamberlain, Petr Pavlu, Sami Tolvanen, Daniel Gomez,
Thorsten Blum, Kees Cook, linux-modules, linux-kernel
In-Reply-To: <20250824064922.41894-1-da.gomez@kernel.org>
On 24/08/2025 08.49, Daniel Gomez wrote:
> This includes a fix as part of the KSPP (Kernel Self Protection Project) to
> replace the deprecated and unsafe strcpy() calls in the kernel parameter string
> handler and sysfs parameters for built-in modules. Single commit, no functional
> changes.
The fix was merged 1 week ago (August 16) and in linux-next since next-20250818.
^ permalink raw reply
* [GIT PULL] Modules fixes for v6.17-rc3
From: Daniel Gomez @ 2025-08-24 6:49 UTC (permalink / raw)
To: Linus Torvalds
Cc: Luis Chamberlain, Petr Pavlu, Sami Tolvanen, Daniel Gomez,
Thorsten Blum, Kees Cook, linux-modules, linux-kernel
The following changes since commit 8f5ae30d69d7543eee0d70083daf4de8fe15d585:
Linux 6.17-rc1 (2025-08-10 19:41:16 +0300)
are available in the Git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/modules/linux.git tags/modules-6.17-rc3.fixes
for you to fetch changes up to 5eb4b9a4cdbb70d70377fe8fb2920b75910e5024:
params: Replace deprecated strcpy() with strscpy() and memcpy() (2025-08-16 21:47:25 +0200)
----------------------------------------------------------------
Modules fixes for 6.17-rc3
This includes a fix as part of the KSPP (Kernel Self Protection Project) to
replace the deprecated and unsafe strcpy() calls in the kernel parameter string
handler and sysfs parameters for built-in modules. Single commit, no functional
changes.
Note that param_ops_charp still contains the last remaining strcpy() call in
the kernel/params.c file.
-----BEGIN PGP SIGNATURE-----
iQIzBAABCAAdFiEE73Ua4R8Pc+G5xjxTQJ6jxB8ZUfsFAmiqshwACgkQQJ6jxB8Z
UfuF8Q//UVMLtvEREUM8m18u1SVDhnO9/+Dpl/bkTof+w03KviIbAN/VXm6qn3C7
ZYtK5lDWU4twg+bOpC6EC/LdNVNyJx6cCpqkZFri21i9Yhf5ak0Sp833lWTZdqH/
DDNCCAOuFH1EaihlJwQ/T4ox1CUOBTC49HyCBnVvdWiCCevUaPbxc8Cgsuzp/gsf
vqXoOJCYKZD2ZdeRKgW7EgETWUljIpvjfXnb3DtMHztj92wzHPaCR50d0iBJbpZi
3JEmrZ6FQ+sb+Qgp4VrW7ZEIa8UFGusqKVZBzJfZR61OU+iVz97gg2WptczsTZCa
GoV0sM5MbNwaBtaMEoM40OiQWCAtYyfsIFmOH142Djcmzgs2hGFTGMKgZReDRs8B
XiPPTq0IW5czYLoNyzJKvtoRX1qBC7wV0rxN9MY8AQieCPmhV3fQsjUgnPKwUOlV
4U5EvzI2Qy4LL5oEUp3rEcymio/rP1wrd0dFxx/D+bMMj//PRF+9rr51deZ/tqtz
Y0Q8rI7CYYlhg0I6XH8t2sAe2TypbU8gNGhOi23Z9vBZwtOv1e3fGTQXymopvVT4
m50c541senApTKFHDbbck2KXwNyasdtIWCQrtChaP99Y0Lk2KRxkgYtVuCJEPNsv
rFjyK3KnfkhhqNJEl4I8EUJwfQ1z9tUHJyfx1p0q74cShV7fRg8=
=8/92
-----END PGP SIGNATURE-----
----------------------------------------------------------------
Thorsten Blum (1):
params: Replace deprecated strcpy() with strscpy() and memcpy()
kernel/params.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
^ permalink raw reply
* Re: [PATCH 1/5] module: Fix module_sig_check() for modules with ignored modversions/vermagic
From: Sami Tolvanen @ 2025-08-22 19:36 UTC (permalink / raw)
To: Jinchao Wang
Cc: Luis Chamberlain, Petr Pavlu, Daniel Gomez, linux-modules,
linux-kernel
In-Reply-To: <20250822125454.1287066-2-wangjinchao600@gmail.com>
On Fri, Aug 22, 2025 at 5:55 AM Jinchao Wang <wangjinchao600@gmail.com> wrote:
>
> The current signature check logic incorrectly fails modules that have
> valid signatures when the caller specifies MODULE_INIT_IGNORE_MODVERSIONS
> or MODULE_INIT_IGNORE_VERMAGIC flags. This happens because the code
> treats these flags as indicating a "mangled module" and skips signature
> verification entirely.
>
> The key insight is that the intent of the caller (to ignore modversions
> or vermagic) should not affect signature verification. A module with
> a valid signature should be verified regardless of whether the caller
> wants to ignore versioning information.
Why would you need to ignore versions when loading signed modules?
Here's the original series that added this check and I feel it's very
much relevant still:
https://lore.kernel.org/lkml/20160423184421.GL3348@decadent.org.uk/
Sami
^ permalink raw reply
* [PATCH 5/5] module: separate vermagic and livepatch checks
From: Jinchao Wang @ 2025-08-22 12:54 UTC (permalink / raw)
To: Luis Chamberlain, Petr Pavlu, Daniel Gomez, Sami Tolvanen,
linux-modules, linux-kernel
Cc: Jinchao Wang
In-Reply-To: <20250822125454.1287066-1-wangjinchao600@gmail.com>
Rename check_modinfo() to check_modinfo_vermagic() to clarify it only
checks vermagic compatibility.
Move livepatch check to happen after vermagic check in early_mod_check(),
creating proper separation of concerns.
This makes the module loading sequence more logical:
- First verify module vermagic
- Then check livepatch-specific requirements
No functional changes, just clearer code organization.
Signed-off-by: Jinchao Wang <wangjinchao600@gmail.com>
---
kernel/module/main.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/kernel/module/main.c b/kernel/module/main.c
index a426bd8a18b5..d30bffeef63e 100644
--- a/kernel/module/main.c
+++ b/kernel/module/main.c
@@ -2571,7 +2571,7 @@ static void module_augment_kernel_taints(struct module *mod, struct load_info *i
}
-static int check_modinfo(struct module *mod, struct load_info *info, int flags)
+static int check_modinfo_vermagic(struct module *mod, struct load_info *info, int flags)
{
const char *modmagic = get_modinfo(info, "vermagic");
int err;
@@ -2590,10 +2590,6 @@ static int check_modinfo(struct module *mod, struct load_info *info, int flags)
return -ENOEXEC;
}
- err = check_modinfo_livepatch(mod, info);
- if (err)
- return err;
-
return 0;
}
@@ -3334,7 +3330,11 @@ static int early_mod_check(struct load_info *info, int flags)
if (!check_modstruct_version(info, info->mod))
return -ENOEXEC;
- err = check_modinfo(info->mod, info, flags);
+ err = check_modinfo_vermagic(info->mod, info, flags);
+ if (err)
+ return err;
+
+ err = check_modinfo_livepatch(info->mod, info);
if (err)
return err;
--
2.43.0
^ permalink raw reply related
* [PATCH 4/5] module: centralize no-versions force load check
From: Jinchao Wang @ 2025-08-22 12:54 UTC (permalink / raw)
To: Luis Chamberlain, Petr Pavlu, Daniel Gomez, Sami Tolvanen,
linux-modules, linux-kernel
Cc: Jinchao Wang
In-Reply-To: <20250822125454.1287066-1-wangjinchao600@gmail.com>
Move try_to_force_load() call from check_version() to
check_modstruct_version() to handle "no versions" case only once before
other version checks.
This prevents duplicate force load attempts and makes the error message
show the proper reason.
Signed-off-by: Jinchao Wang <wangjinchao600@gmail.com>
---
kernel/module/version.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/kernel/module/version.c b/kernel/module/version.c
index 2beefeba82d9..3f07fd03cb30 100644
--- a/kernel/module/version.c
+++ b/kernel/module/version.c
@@ -41,10 +41,6 @@ int check_version(const struct load_info *info,
return 1;
}
- /* No versions at all? modprobe --force does this. */
- if (versindex == 0)
- return try_to_force_load(mod, symname) == 0;
-
versions = (void *)sechdrs[versindex].sh_addr;
num_versions = sechdrs[versindex].sh_size
/ sizeof(struct modversion_info);
@@ -81,6 +77,11 @@ int check_modstruct_version(const struct load_info *info,
};
bool have_symbol;
+ /* No versions at all? modprobe --force does this. */
+ if (info->index.vers == 0 &&
+ try_to_force_load(mod, "no versions module"))
+ return 1;
+
/*
* Since this should be found in kernel (which can't be removed), no
* locking is necessary. Regardless use a RCU read section to keep
--
2.43.0
^ permalink raw reply related
* [PATCH 3/5] module: show why force load fails
From: Jinchao Wang @ 2025-08-22 12:54 UTC (permalink / raw)
To: Luis Chamberlain, Petr Pavlu, Daniel Gomez, Sami Tolvanen,
linux-modules, linux-kernel
Cc: Jinchao Wang
In-Reply-To: <20250822125454.1287066-1-wangjinchao600@gmail.com>
Include reason in error message when force loading is disabled.
Signed-off-by: Jinchao Wang <wangjinchao600@gmail.com>
---
kernel/module/main.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/kernel/module/main.c b/kernel/module/main.c
index c66b26184936..a426bd8a18b5 100644
--- a/kernel/module/main.c
+++ b/kernel/module/main.c
@@ -1083,6 +1083,7 @@ int try_to_force_load(struct module *mod, const char *reason)
add_taint_module(mod, TAINT_FORCED_MODULE, LOCKDEP_NOW_UNRELIABLE);
return 0;
#else
+ pr_err("%s force load is not supported\n", reason);
return -ENOEXEC;
#endif
}
--
2.43.0
^ permalink raw reply related
* [PATCH 2/5] module: signing: Use pr_err for signature rejection
From: Jinchao Wang @ 2025-08-22 12:54 UTC (permalink / raw)
To: Luis Chamberlain, Petr Pavlu, Daniel Gomez, Sami Tolvanen,
linux-modules, linux-kernel
Cc: Jinchao Wang
In-Reply-To: <20250822125454.1287066-1-wangjinchao600@gmail.com>
Make module signature rejection messages more visible by using pr_err
instead of pr_notice.
Signed-off-by: Jinchao Wang <wangjinchao600@gmail.com>
---
kernel/module/signing.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/kernel/module/signing.c b/kernel/module/signing.c
index 9e24c79499de..00f079cafd4f 100644
--- a/kernel/module/signing.c
+++ b/kernel/module/signing.c
@@ -112,7 +112,7 @@ int module_sig_check(struct load_info *info, int flags)
}
if (is_module_sig_enforced()) {
- pr_notice("Loading of %s is rejected\n", reason);
+ pr_err("Loading of %s is rejected\n", reason);
return -EKEYREJECTED;
}
--
2.43.0
^ permalink raw reply related
* [PATCH 1/5] module: Fix module_sig_check() for modules with ignored modversions/vermagic
From: Jinchao Wang @ 2025-08-22 12:54 UTC (permalink / raw)
To: Luis Chamberlain, Petr Pavlu, Daniel Gomez, Sami Tolvanen,
linux-modules, linux-kernel
Cc: Jinchao Wang
In-Reply-To: <20250822125454.1287066-1-wangjinchao600@gmail.com>
The current signature check logic incorrectly fails modules that have
valid signatures when the caller specifies MODULE_INIT_IGNORE_MODVERSIONS
or MODULE_INIT_IGNORE_VERMAGIC flags. This happens because the code
treats these flags as indicating a "mangled module" and skips signature
verification entirely.
The key insight is that the intent of the caller (to ignore modversions
or vermagic) should not affect signature verification. A module with
a valid signature should be verified regardless of whether the caller
wants to ignore versioning information.
The signature represents the authenticity and integrity of the module
content, which is independent of version compatibility checks. By
removing the mangled_module check, we allow signature verification to
proceed for modules that have both valid signatures and are being loaded
with version checking disabled.
This fixes cases where modules with correct signatures were being
rejected when loaded with modversions or vermagic ignored, even though
the signature itself was valid and should have been verified.
Signed-off-by: Jinchao Wang <wangjinchao600@gmail.com>
---
kernel/module/signing.c | 13 ++++---------
1 file changed, 4 insertions(+), 9 deletions(-)
diff --git a/kernel/module/signing.c b/kernel/module/signing.c
index a2ff4242e623..9e24c79499de 100644
--- a/kernel/module/signing.c
+++ b/kernel/module/signing.c
@@ -73,15 +73,10 @@ int module_sig_check(struct load_info *info, int flags)
const unsigned long markerlen = sizeof(MODULE_SIG_STRING) - 1;
const char *reason;
const void *mod = info->hdr;
- bool mangled_module = flags & (MODULE_INIT_IGNORE_MODVERSIONS |
- MODULE_INIT_IGNORE_VERMAGIC);
- /*
- * Do not allow mangled modules as a module with version information
- * removed is no longer the module that was signed.
- */
- if (!mangled_module &&
- info->len > markerlen &&
- memcmp(mod + info->len - markerlen, MODULE_SIG_STRING, markerlen) == 0) {
+
+ if (info->len > markerlen &&
+ memcmp(mod + info->len - markerlen, MODULE_SIG_STRING, markerlen) ==
+ 0) {
/* We truncate the module to discard the signature */
info->len -= markerlen;
err = mod_verify_sig(mod, info);
--
2.43.0
^ permalink raw reply related
* [PATCH 0/5] Module loading error handling improvements
From: Jinchao Wang @ 2025-08-22 12:54 UTC (permalink / raw)
To: Luis Chamberlain, Petr Pavlu, Daniel Gomez, Sami Tolvanen,
linux-modules, linux-kernel
Cc: Jinchao Wang
This series addresses several issues related to module loading error
handling, particularly around force loading and signature verification.
The most critical fix is in patch 1, which resolves a bug where signed
modules were incorrectly rejected when loaded with the -f flag (force
load).
The others improve the user experience when troubleshooting
module loading issues while maintaining the security guarantees of
module signing.
Jinchao Wang (5):
module: Fix module_sig_check() for modules with ignored modversions/vermagic
module: signing: Use pr_err for signature rejection
module: show why force load fails
module: centralize no-versions force load check
module: separate vermagic and livepatch checks
kernel/module/main.c | 13 +++++++------
kernel/module/signing.c | 15 +++++----------
kernel/module/version.c | 9 +++++----
3 files changed, 17 insertions(+), 20 deletions(-)
--
2.43.0
^ permalink raw reply
* Re: [PATCH] module: Remove unused __INIT*_OR_MODULE macros
From: Sami Tolvanen @ 2025-08-21 23:11 UTC (permalink / raw)
To: Petr Pavlu; +Cc: Luis Chamberlain, Daniel Gomez, linux-modules, linux-kernel
In-Reply-To: <20250819121423.460156-1-petr.pavlu@suse.com>
On Tue, Aug 19, 2025 at 5:14 AM Petr Pavlu <petr.pavlu@suse.com> wrote:
>
> Remove the __INIT_OR_MODULE, __INITDATA_OR_MODULE and
> __INITRODATA_OR_MODULE macros. These were introduced in commit 8b5a10fc6fd0
> ("x86: properly annotate alternatives.c"). Only __INITRODATA_OR_MODULE was
> ever used, in arch/x86/kernel/alternative.c. In 2011, commit dc326fca2b64
> ("x86, cpu: Clean up and unify the NOP selection infrastructure") removed
> this usage.
>
> Signed-off-by: Petr Pavlu <petr.pavlu@suse.com>
Reviewed-by: Sami Tolvanen <samitolvanen@google.com>
Sami
^ permalink raw reply
* Re: [PATCH] params: Replace __modinit with __init_or_module
From: Sami Tolvanen @ 2025-08-21 23:10 UTC (permalink / raw)
To: Petr Pavlu
Cc: Luis Chamberlain, Daniel Gomez, Shyam Saini, Rasmus Villemoes,
linux-modules, linux-kernel
In-Reply-To: <20250819121248.460105-1-petr.pavlu@suse.com>
On Tue, Aug 19, 2025 at 5:13 AM Petr Pavlu <petr.pavlu@suse.com> wrote:
>
> Remove the custom __modinit macro from kernel/params.c and instead use the
> common __init_or_module macro from include/linux/module.h. Both provide the
> same functionality.
>
> Signed-off-by: Petr Pavlu <petr.pavlu@suse.com>
Reviewed-by: Sami Tolvanen <samitolvanen@google.com>
Sami
^ permalink raw reply
* Re: [PATCH v3 8/8] x86/ftrace: enable EXECMEM_ROX_CACHE for ftrace allocations
From: Steven Rostedt @ 2025-08-21 19:25 UTC (permalink / raw)
To: Mike Rapoport
Cc: Andrew Morton, Andy Lutomirski, Borislav Petkov, Christophe Leroy,
Daniel Gomez, Dave Hansen, Ingo Molnar, Liam R. Howlett,
Luis Chamberlain, Mark Rutland, Masami Hiramatsu, H. Peter Anvin,
Peter Zijlstra, Petr Pavlu, Sami Tolvanen, Thomas Gleixner,
Yann Ylavic, linux-kernel, linux-mm, linux-modules,
linux-trace-kernel, x86
In-Reply-To: <aKa4otIF6AbhD2X3@kernel.org>
On Thu, 21 Aug 2025 09:11:46 +0300
Mike Rapoport <rppt@kernel.org> wrote:
> maple tree is initialized after ftrace, so the patch below should fix it:
>
> diff --git a/init/main.c b/init/main.c
> index 0ee0ee7b7c2c..5753e9539ae6 100644
> --- a/init/main.c
> +++ b/init/main.c
> @@ -956,6 +956,7 @@ void start_kernel(void)
> sort_main_extable();
> trap_init();
> mm_core_init();
> + maple_tree_init();
> poking_init();
> ftrace_init();
>
> @@ -973,7 +974,6 @@ void start_kernel(void)
> "Interrupts were enabled *very* early, fixing it\n"))
> local_irq_disable();
> radix_tree_init();
> - maple_tree_init();
>
> /*
> * Set up housekeeping before setting up workqueues to allow the unbound
>
Tested-by: Steven Rostedt (Google) <rostedt@goodmis.org>
Thanks,
-- Steve
^ permalink raw reply
* Re: Question: a module for wiping userspace RAM before shutdown/reboot/halt
From: Kamil Aronowski @ 2025-08-21 12:13 UTC (permalink / raw)
To: Danill Klimuk, linux-modules
In-Reply-To: <bfe72929-ba4c-4732-9f80-25cc7b95a0c8@3mdeb.com>
[-- Attachment #1.1.1: Type: text/plain, Size: 2373 bytes --]
Recently, we evaluated the effectiveness of the `init_on_free`
mechanism, particularly in the context of preserving privacy by
clearing RAM for individuals with high operational security
requirements.
As mentioned
(https://lore.kernel.org/all/e71bd62c-5ba7-4363-9af1-d9c9de394a54@3mdeb.com/),
we'd like to ensure that our clients do not have their confidential
data leaked after their session has ended with a shutdown/reboot/halt.
In short, `init_on_free` appears to wipe the LUKS secret key
successfully, but some non-kernel space snippets remain in memory.
Some tests have been performed by dumping memory after booting Debian
13 (with `init_on_free` enabled) and then rebooting to our custom EFI
memory dumping application. For instance, the mentions of
`apparmor_parser`, XKB, udev, or systemd units have been found in the
memory dump:
```
audit: type=1400 audit(1755156467.556:2): apparmor="STATUS"
operation="profile_load" profile="unconfined" name="Discord" pid=967
comm="apparmor_parser"r"
[...]
partial alphanumeric_keys
xkb_symbols "tib_asciinum" {
include "cn(tib)"
name[Group1]= "Tibetan (with ASCII numerals)";
key <AE01> { [ 1, 0x1000f21, 0x1000f04, 0x1000f76 ] }; # 1
[...]
I:10114000
E:ID_MM_CANDIDATE=1
S:disk/by-id/dm-uuid-CRYPT-LUKS2-00b4b79c209a4dcfadf37e310778f583-sda3_crypt
[...]
[Unit]
Description=Switch Root
AssertPathExists=/etc/initrd-release
DefaultDependencies=no
Wants=initrd-switch-root.service
Before=initrd-switch-root.service
AllowIsolate=yes
Wants=initrd-udevadm-cleanup-db.service initrd-root-fs.target
initrd-fs.target systemd-journald.service initrd-cleanup.service
After=initrd-udevadm-cleanup-db.service initrd-root-fs.target
initrd-fs.target emergency.service emergency.target initrd-cleanup.service
[...]
```
Is this the expected behavior, a bug, or a misconfiguration on our
end?
If it is indeed a bug, we'd be happy to cooperate on improving the
`init_on_free` mechanism. If it is expected behavior than we will
consider wiping userspace memory some other way, e.g. by implementing
a separate Linux Kernel module as described in the previous email
(https://lore.kernel.org/all/e71bd62c-5ba7-4363-9af1-d9c9de394a54@3mdeb.com/).
--
Kamil Aronowski
Junior Embedded Firmware Engineer
GPG: 3510148A5CD67908
https://3mdeb.com | @3mdeb_com
[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 5529 bytes --]
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH v3 8/8] x86/ftrace: enable EXECMEM_ROX_CACHE for ftrace allocations
From: Mike Rapoport @ 2025-08-21 6:11 UTC (permalink / raw)
To: Steven Rostedt
Cc: Andrew Morton, Andy Lutomirski, Borislav Petkov, Christophe Leroy,
Daniel Gomez, Dave Hansen, Ingo Molnar, Liam R. Howlett,
Luis Chamberlain, Mark Rutland, Masami Hiramatsu, H. Peter Anvin,
Peter Zijlstra, Petr Pavlu, Sami Tolvanen, Thomas Gleixner,
Yann Ylavic, linux-kernel, linux-mm, linux-modules,
linux-trace-kernel, x86
In-Reply-To: <20250820184743.0302a8b5@gandalf.local.home>
On Wed, Aug 20, 2025 at 06:47:43PM -0400, Steven Rostedt wrote:
> On Sun, 13 Jul 2025 10:17:30 +0300
> Mike Rapoport <rppt@kernel.org> wrote:
>
> > From: "Mike Rapoport (Microsoft)" <rppt@kernel.org>
> >
> > For the most part ftrace uses text poking and can handle ROX memory.
> > The only place that requires writable memory is create_trampoline() that
> > updates the allocated memory and in the end makes it ROX.
> >
> > Use execmem_alloc_rw() in x86::ftrace::alloc_tramp() and enable ROX cache
> > for EXECMEM_FTRACE when configuration and CPU features allow that.
> >
> > Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> > Signed-off-by: Mike Rapoport (Microsoft) <rppt@kernel.org>
> > ---
>
> The "ftrace=function" kernel command line started crashing with v6.17-rc1,
> and I bisected it down to this commit:
>
> 5d79c2be5081 ("x86/ftrace: enable EXECMEM_ROX_CACHE for ftrace allocations")
>
> On boot I hit this:
>
> [ 0.159269] BUG: kernel NULL pointer dereference, address: 000000000000001c
> [ 0.160254] #PF: supervisor read access in kernel mode
> [ 0.160975] #PF: error_code(0x0000) - not-present page
> [ 0.161697] PGD 0 P4D 0
> [ 0.162055] Oops: Oops: 0000 [#1] SMP PTI
> [ 0.162619] CPU: 0 UID: 0 PID: 0 Comm: swapper Not tainted 6.17.0-rc2-test-00006-g48d06e78b7cb-dirty #9 PREEMPT(undef)
> [ 0.164141] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.3-debian-1.16.3-2 04/01/2014
> [ 0.165439] RIP: 0010:kmem_cache_alloc_noprof (mm/slub.c:4237)
> [ 0.177483] Call Trace:
> [ 0.177828] <TASK>
> [ 0.178123] mas_alloc_nodes (lib/maple_tree.c:176 (discriminator 2) lib/maple_tree.c:1255 (discriminator 2))
> [ 0.178692] mas_store_gfp (lib/maple_tree.c:5468)
> [ 0.179223] execmem_cache_add_locked (mm/execmem.c:207)
> [ 0.179870] execmem_alloc (mm/execmem.c:213 mm/execmem.c:313 mm/execmem.c:335 mm/execmem.c:475)
> [ 0.180397] ? ftrace_caller (arch/x86/kernel/ftrace_64.S:169)
> [ 0.180922] ? __pfx_ftrace_caller (arch/x86/kernel/ftrace_64.S:158)
> [ 0.181517] execmem_alloc_rw (mm/execmem.c:487)
> [ 0.182052] arch_ftrace_update_trampoline (arch/x86/kernel/ftrace.c:266 arch/x86/kernel/ftrace.c:344 arch/x86/kernel/ftrace.c:474)
> [ 0.182778] ? ftrace_caller_op_ptr (arch/x86/kernel/ftrace_64.S:182)
> [ 0.183388] ftrace_update_trampoline (kernel/trace/ftrace.c:7947)
> [ 0.184024] __register_ftrace_function (kernel/trace/ftrace.c:368)
> [ 0.184682] ftrace_startup (kernel/trace/ftrace.c:3048)
> [ 0.185205] ? __pfx_function_trace_call (kernel/trace/trace_functions.c:210)
> [ 0.185877] register_ftrace_function_nolock (kernel/trace/ftrace.c:8717)
> [ 0.186595] register_ftrace_function (kernel/trace/ftrace.c:8745)
> [ 0.187254] ? __pfx_function_trace_call (kernel/trace/trace_functions.c:210)
> [ 0.187924] function_trace_init (kernel/trace/trace_functions.c:170)
> [ 0.188499] tracing_set_tracer (kernel/trace/trace.c:5916 kernel/trace/trace.c:6349)
> [ 0.189088] register_tracer (kernel/trace/trace.c:2391)
> [ 0.189642] early_trace_init (kernel/trace/trace.c:11075 kernel/trace/trace.c:11149)
> [ 0.190204] start_kernel (init/main.c:970)
> [ 0.190732] x86_64_start_reservations (arch/x86/kernel/head64.c:307)
> [ 0.191381] x86_64_start_kernel (??:?)
> [ 0.191955] common_startup_64 (arch/x86/kernel/head_64.S:419)
> [ 0.192534] </TASK>
> [ 0.192839] Modules linked in:
> [ 0.193267] CR2: 000000000000001c
> [ 0.193730] ---[ end trace 0000000000000000 ]---
maple tree is initialized after ftrace, so the patch below should fix it:
diff --git a/init/main.c b/init/main.c
index 0ee0ee7b7c2c..5753e9539ae6 100644
--- a/init/main.c
+++ b/init/main.c
@@ -956,6 +956,7 @@ void start_kernel(void)
sort_main_extable();
trap_init();
mm_core_init();
+ maple_tree_init();
poking_init();
ftrace_init();
@@ -973,7 +974,6 @@ void start_kernel(void)
"Interrupts were enabled *very* early, fixing it\n"))
local_irq_disable();
radix_tree_init();
- maple_tree_init();
/*
* Set up housekeeping before setting up workqueues to allow the unbound
> -- Steve
--
Sincerely yours,
Mike.
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox