Re: [PATCH v2 0/3] sysfs: constify bin_attribute argument of sysfs_bin_attr_simple_read()
On Thu, Jan 09, 2025 at 08:56:37AM +0100, Greg Kroah-Hartman wrote: > The "pointless" penalty will go away once we convert all instances, and > really, it's just one pointer check, sysfs files should NOT be a hot > path for anything real, and one more pointer check should be cached and > not measurable compared to the real logic behind the binary data coming > from the hardware/kernel, right? > > sysfs is NOT tuned for speed at all, so adding more checks like this > should be fine. Hey, when I duplicated the method to convert sysfs over to a proper seq_file based approach that avoids buffer overflows you basically came up with the same line that Alexei had here. And that is a lot more useful than constification. Not that I mind the latter, but it would be better if it could be done without leaving both variants in for long.
[syzbot] [virt?] [net?] general protection fault in vsock_connectible_has_data
Hello,
syzbot found the following issue on:
HEAD commit:8ce4f287524c net: libwx: fix firmware mailbox abnormal ret..
git tree: net
console+strace: https://syzkaller.appspot.com/x/log.txt?x=13f06edf98
kernel config: https://syzkaller.appspot.com/x/.config?x=1c541fa8af5c9cc7
dashboard link: https://syzkaller.appspot.com/bug?extid=3affdbfc986ecd9200fd
compiler: Debian clang version 15.0.6, GNU ld (GNU Binutils for Debian)
2.40
syz repro: https://syzkaller.appspot.com/x/repro.syz?x=1569541858
C reproducer: https://syzkaller.appspot.com/x/repro.c?x=124c56f858
Downloadable assets:
disk image:
https://storage.googleapis.com/syzbot-assets/e09bf4b8939b/disk-8ce4f287.raw.xz
vmlinux:
https://storage.googleapis.com/syzbot-assets/f7f7846f83db/vmlinux-8ce4f287.xz
kernel image:
https://storage.googleapis.com/syzbot-assets/44540dea47ac/bzImage-8ce4f287.xz
The issue was bisected to:
commit 69139d2919dd4aa9a553c8245e7c63e82613e3fc
Author: Cong Wang
Date: Mon Aug 12 02:21:53 2024 +
vsock: fix recursive ->recvmsg calls
bisection log: https://syzkaller.appspot.com/x/bisect.txt?x=116bc4b058
final oops: https://syzkaller.appspot.com/x/report.txt?x=136bc4b058
console output: https://syzkaller.appspot.com/x/log.txt?x=156bc4b058
IMPORTANT: if you fix the issue, please add the following tag to the commit:
Reported-by: syzbot+3affdbfc986ecd920...@syzkaller.appspotmail.com
Fixes: 69139d2919dd ("vsock: fix recursive ->recvmsg calls")
Oops: general protection fault, probably for non-canonical address
0xdc14: [#1] PREEMPT SMP KASAN PTI
KASAN: null-ptr-deref in range [0x00a0-0x00a7]
CPU: 1 UID: 0 PID: 5828 Comm: syz-executor976 Not tainted
6.13.0-rc5-syzkaller-00142-g8ce4f287524c #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google
09/13/2024
RIP: 0010:vsock_connectible_has_data+0x85/0x100 net/vmw_vsock/af_vsock.c:882
Code: 80 3c 38 00 74 08 48 89 df e8 e7 e0 5f f6 48 8b 1b 66 83 fd 05 75 3a e8
d9 78 f9 f5 48 81 c3 a0 00 00 00 48 89 d8 48 c1 e8 03 <42> 80 3c 38 00 74 08 48
89 df e8 bc e0 5f f6 4c 8b 1b 4c 89 f7 41
RSP: 0018:c900015976f8 EFLAGS: 00010206
RAX: 0014 RBX: 00a0 RCX: 888033e09e00
RDX: RSI: 0005 RDI: 0005
RBP: 0005 R08: 8ba5fadc R09: 1285492b
R10: dc00 R11: fbfff285492c R12: 2000
R13: dc00 R14: 888033e18000 R15: dc00
FS: 565ca380() GS:8880b870() knlGS:
CS: 0010 DS: ES: CR0: 80050033
CR2: 200061c8 CR3: 74f74000 CR4: 003526f0
DR0: DR1: DR2:
DR3: DR6: fffe0ff0 DR7: 0400
Call Trace:
vsock_has_data net/vmw_vsock/vsock_bpf.c:30 [inline]
vsock_bpf_recvmsg+0x4b5/0x10a0 net/vmw_vsock/vsock_bpf.c:87
sock_recvmsg_nosec net/socket.c:1033 [inline]
sock_recvmsg+0x22f/0x280 net/socket.c:1055
sys_recvmsg+0x1c6/0x480 net/socket.c:2803
___sys_recvmsg net/socket.c:2845 [inline]
do_recvmmsg+0x426/0xab0 net/socket.c:2940
__sys_recvmmsg net/socket.c:3014 [inline]
__do_sys_recvmmsg net/socket.c:3037 [inline]
__se_sys_recvmmsg net/socket.c:3030 [inline]
__x64_sys_recvmmsg+0x199/0x250 net/socket.c:3030
do_syscall_x64 arch/x86/entry/common.c:52 [inline]
do_syscall_64+0xf3/0x230 arch/x86/entry/common.c:83
entry_SYSCALL_64_after_hwframe+0x77/0x7f
RIP: 0033:0x7fb38b2465e9
Code: 48 83 c4 28 c3 e8 37 17 00 00 0f 1f 80 00 00 00 00 48 89 f8 48 89 f7 48
89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01
c3 48 c7 c1 b8 ff ff ff f7 d8 64 89 01 48
RSP: 002b:7fffd43f6938 EFLAGS: 0246 ORIG_RAX: 012b
RAX: ffda RBX: 7fffd43f6b08 RCX: 7fb38b2465e9
RDX: 0001 RSI: 200061c0 RDI: 0003
RBP: 7fb38b2b9610 R08: R09: 7fffd43f6b08
R10: 2000 R11: 0246 R12: 0001
R13: 7fffd43f6af8 R14: 0001 R15: 0001
Modules linked in:
---[ end trace ]---
RIP: 0010:vsock_connectible_has_data+0x85/0x100 net/vmw_vsock/af_vsock.c:882
Code: 80 3c 38 00 74 08 48 89 df e8 e7 e0 5f f6 48 8b 1b 66 83 fd 05 75 3a e8
d9 78 f9 f5 48 81 c3 a0 00 00 00 48 89 d8 48 c1 e8 03 <42> 80 3c 38 00 74 08 48
89 df e8 bc e0 5f f6 4c 8b 1b 4c 89 f7 41
RSP: 0018:c900015976f8 EFLAGS: 00010206
RAX: 0014 RBX: 00a0 RCX: 888033e09e00
RDX: RSI: 0005 RDI: 0005
RBP: 0005 R08: 8ba5fadc R09: 1285492b
R10: dc00 R11: fbfff285492c R12: 2000
R13: dc00 R14: 888033e18000 R15: dc00
FS: 565ca380() GS:8880b860() knlGS:
CS: 0010 DS: ES: CR
Re: [PATCH v2 0/3] sysfs: constify bin_attribute argument of sysfs_bin_attr_simple_read()
On Thu, Jan 09, 2025 at 12:06:09AM -0800, Christoph Hellwig wrote: > On Thu, Jan 09, 2025 at 08:56:37AM +0100, Greg Kroah-Hartman wrote: > > The "pointless" penalty will go away once we convert all instances, and > > really, it's just one pointer check, sysfs files should NOT be a hot > > path for anything real, and one more pointer check should be cached and > > not measurable compared to the real logic behind the binary data coming > > from the hardware/kernel, right? > > > > sysfs is NOT tuned for speed at all, so adding more checks like this > > should be fine. > > Hey, when I duplicated the method to convert sysfs over to a proper > seq_file based approach that avoids buffer overflows you basically > came up with the same line that Alexei had here. I did? Sorry about that, I don't remember that. > And that is a lot > more useful than constification. Not that I mind the latter, but it > would be better if it could be done without leaving both variants > in for long. I agree, we should get the read_new stuff out in the next kernel cycle I hope. As for seq_file for sysfs, is that for binary attributes only, or for all? I can't recall that at all. thanks, greg k-h
Re: [PATCH v2 0/3] sysfs: constify bin_attribute argument of sysfs_bin_attr_simple_read()
On Thu, Jan 09, 2025 at 09:12:03AM +0100, Greg Kroah-Hartman wrote: > > Hey, when I duplicated the method to convert sysfs over to a proper > > seq_file based approach that avoids buffer overflows you basically > > came up with the same line that Alexei had here. > > I did? Sorry about that, I don't remember that. It's been a while.. > As for seq_file for sysfs, is that for binary attributes only, or for > all? I can't recall that at all. Non-binary ones.
Re: [syzbot] [virt?] [net?] general protection fault in vsock_connectible_has_data
On Thu, Jan 09, 2025 at 12:11:20AM -0800, syzbot wrote:
Hello,
syzbot found the following issue on:
HEAD commit:8ce4f287524c net: libwx: fix firmware mailbox abnormal ret..
git tree: net
console+strace: https://syzkaller.appspot.com/x/log.txt?x=13f06edf98
kernel config: https://syzkaller.appspot.com/x/.config?x=1c541fa8af5c9cc7
dashboard link: https://syzkaller.appspot.com/bug?extid=3affdbfc986ecd9200fd
compiler: Debian clang version 15.0.6, GNU ld (GNU Binutils for Debian)
2.40
syz repro: https://syzkaller.appspot.com/x/repro.syz?x=1569541858
C reproducer: https://syzkaller.appspot.com/x/repro.c?x=124c56f858
Downloadable assets:
disk image:
https://storage.googleapis.com/syzbot-assets/e09bf4b8939b/disk-8ce4f287.raw.xz
vmlinux:
https://storage.googleapis.com/syzbot-assets/f7f7846f83db/vmlinux-8ce4f287.xz
kernel image:
https://storage.googleapis.com/syzbot-assets/44540dea47ac/bzImage-8ce4f287.xz
The issue was bisected to:
commit 69139d2919dd4aa9a553c8245e7c63e82613e3fc
Author: Cong Wang
Date: Mon Aug 12 02:21:53 2024 +
vsock: fix recursive ->recvmsg calls
bisection log: https://syzkaller.appspot.com/x/bisect.txt?x=116bc4b058
final oops: https://syzkaller.appspot.com/x/report.txt?x=136bc4b058
console output: https://syzkaller.appspot.com/x/log.txt?x=156bc4b058
IMPORTANT: if you fix the issue, please add the following tag to the commit:
Reported-by: syzbot+3affdbfc986ecd920...@syzkaller.appspotmail.com
Fixes: 69139d2919dd ("vsock: fix recursive ->recvmsg calls")
Oops: general protection fault, probably for non-canonical address
0xdc14: [#1] PREEMPT SMP KASAN PTI
KASAN: null-ptr-deref in range [0x00a0-0x00a7]
CPU: 1 UID: 0 PID: 5828 Comm: syz-executor976 Not tainted
6.13.0-rc5-syzkaller-00142-g8ce4f287524c #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google
09/13/2024
RIP: 0010:vsock_connectible_has_data+0x85/0x100 net/vmw_vsock/af_vsock.c:882
Code: 80 3c 38 00 74 08 48 89 df e8 e7 e0 5f f6 48 8b 1b 66 83 fd 05 75 3a e8 d9 78
f9 f5 48 81 c3 a0 00 00 00 48 89 d8 48 c1 e8 03 <42> 80 3c 38 00 74 08 48 89 df
e8 bc e0 5f f6 4c 8b 1b 4c 89 f7 41
RSP: 0018:c900015976f8 EFLAGS: 00010206
RAX: 0014 RBX: 00a0 RCX: 888033e09e00
RDX: RSI: 0005 RDI: 0005
RBP: 0005 R08: 8ba5fadc R09: 1285492b
R10: dc00 R11: fbfff285492c R12: 2000
R13: dc00 R14: 888033e18000 R15: dc00
FS: 565ca380() GS:8880b870() knlGS:
CS: 0010 DS: ES: CR0: 80050033
CR2: 200061c8 CR3: 74f74000 CR4: 003526f0
DR0: DR1: DR2:
DR3: DR6: fffe0ff0 DR7: 0400
Call Trace:
vsock_has_data net/vmw_vsock/vsock_bpf.c:30 [inline]
vsock_bpf_recvmsg+0x4b5/0x10a0 net/vmw_vsock/vsock_bpf.c:87
sock_recvmsg_nosec net/socket.c:1033 [inline]
sock_recvmsg+0x22f/0x280 net/socket.c:1055
sys_recvmsg+0x1c6/0x480 net/socket.c:2803
___sys_recvmsg net/socket.c:2845 [inline]
do_recvmmsg+0x426/0xab0 net/socket.c:2940
__sys_recvmmsg net/socket.c:3014 [inline]
__do_sys_recvmmsg net/socket.c:3037 [inline]
__se_sys_recvmmsg net/socket.c:3030 [inline]
__x64_sys_recvmmsg+0x199/0x250 net/socket.c:3030
do_syscall_x64 arch/x86/entry/common.c:52 [inline]
do_syscall_64+0xf3/0x230 arch/x86/entry/common.c:83
entry_SYSCALL_64_after_hwframe+0x77/0x7f
RIP: 0033:0x7fb38b2465e9
Code: 48 83 c4 28 c3 e8 37 17 00 00 0f 1f 80 00 00 00 00 48 89 f8 48 89 f7 48 89 d6
48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48
c7 c1 b8 ff ff ff f7 d8 64 89 01 48
RSP: 002b:7fffd43f6938 EFLAGS: 0246 ORIG_RAX: 012b
RAX: ffda RBX: 7fffd43f6b08 RCX: 7fb38b2465e9
RDX: 0001 RSI: 200061c0 RDI: 0003
RBP: 7fb38b2b9610 R08: R09: 7fffd43f6b08
R10: 2000 R11: 0246 R12: 0001
R13: 7fffd43f6af8 R14: 0001 R15: 0001
Modules linked in:
---[ end trace ]---
RIP: 0010:vsock_connectible_has_data+0x85/0x100 net/vmw_vsock/af_vsock.c:882
Code: 80 3c 38 00 74 08 48 89 df e8 e7 e0 5f f6 48 8b 1b 66 83 fd 05 75 3a e8 d9 78
f9 f5 48 81 c3 a0 00 00 00 48 89 d8 48 c1 e8 03 <42> 80 3c 38 00 74 08 48 89 df
e8 bc e0 5f f6 4c 8b 1b 4c 89 f7 41
RSP: 0018:c900015976f8 EFLAGS: 00010206
RAX: 0014 RBX: 00a0 RCX: 888033e09e00
RDX: RSI: 0005 RDI: 0005
RBP: 0005 R08: 8ba5fadc R09: 1285492b
R10: dc00 R11: fbfff285492c R12: 2000
R13: dc00 R14: 888033e18000 R15: dc00
FS: 565ca380() GS:8880b860() knlGS:00
Re: [sos-linux-ext-patches] [PATCH v4 2/8] KVM: selftests: SEV-SNP test for KVM_SEV_INIT2
On 11/15/2024 5:10 AM, Pratik R. Sampat wrote:
> Add the X86_FEATURE_SNP CPU feature to the architectural definition for
> the SEV-SNP VM type to exercise the KVM_SEV_INIT2 call. Ensure that the
> SNP test is skipped in scenarios where CPUID supports it but KVM does
> not, so that a failure is not reported in such cases.
>
> Signed-off-by: Pratik R. Sampat
> ---
> .../selftests/kvm/include/x86_64/processor.h| 1 +
> .../testing/selftests/kvm/x86_64/sev_init2_tests.c | 13 +
> 2 files changed, 14 insertions(+)
>
> diff --git a/tools/testing/selftests/kvm/include/x86_64/processor.h
> b/tools/testing/selftests/kvm/include/x86_64/processor.h
> index 645200e95f89..c18d2405798f 100644
> --- a/tools/testing/selftests/kvm/include/x86_64/processor.h
> +++ b/tools/testing/selftests/kvm/include/x86_64/processor.h
> @@ -199,6 +199,7 @@ struct kvm_x86_cpu_feature {
> #define X86_FEATURE_VGIFKVM_X86_CPU_FEATURE(0x800A,
> 0, EDX, 16)
> #define X86_FEATURE_SEV KVM_X86_CPU_FEATURE(0x801F,
> 0, EAX, 1)
Unrelated to your patch, X86_FEATURE_SEV does not match the KVM API definitions,
1 is used for KVM_X86_SW_PROTECTED_VM
#define KVM_X86_DEFAULT_VM0
#define KVM_X86_SW_PROTECTED_VM 1
#define KVM_X86_SEV_VM2
#define KVM_X86_SEV_ES_VM 3
> #define X86_FEATURE_SEV_ES KVM_X86_CPU_FEATURE(0x801F, 0, EAX,
> 3)
> +#define X86_FEATURE_SNP KVM_X86_CPU_FEATURE(0x801F,
> 0, EAX, 4)
Additionally, please add KVM_X86_SNP_VM to the Documentation/virt/kvm/api.rst
Regards
Nikunj
Re: [PATCH 3/6] syscall.h: introduce syscall_set_nr()
"Dmitry V. Levin" writes:
> Similar to syscall_set_arguments() that complements
> syscall_get_arguments(), introduce syscall_set_nr()
> that complements syscall_get_nr().
>
> syscall_set_nr() is going to be needed along with
> syscall_set_arguments() on all HAVE_ARCH_TRACEHOOK
> architectures to implement PTRACE_SET_SYSCALL_INFO API.
>
> Signed-off-by: Dmitry V. Levin
> ---
> arch/arc/include/asm/syscall.h| 6 ++
> arch/arm/include/asm/syscall.h| 12
> arch/arm64/include/asm/syscall.h | 7 +++
> arch/hexagon/include/asm/syscall.h| 7 +++
> arch/loongarch/include/asm/syscall.h | 7 +++
> arch/m68k/include/asm/syscall.h | 7 +++
> arch/microblaze/include/asm/syscall.h | 7 +++
> arch/mips/include/asm/syscall.h | 7 +++
> arch/nios2/include/asm/syscall.h | 5 +
> arch/openrisc/include/asm/syscall.h | 6 ++
> arch/parisc/include/asm/syscall.h | 7 +++
> arch/powerpc/include/asm/syscall.h| 5 +
> arch/riscv/include/asm/syscall.h | 7 +++
> arch/s390/include/asm/syscall.h | 7 +++
> arch/sh/include/asm/syscall_32.h | 7 +++
> arch/sparc/include/asm/syscall.h | 7 +++
> arch/um/include/asm/syscall-generic.h | 5 +
> arch/x86/include/asm/syscall.h| 7 +++
> arch/xtensa/include/asm/syscall.h | 7 +++
> include/asm-generic/syscall.h | 14 ++
> 20 files changed, 144 insertions(+)
>
> diff --git a/arch/s390/include/asm/syscall.h b/arch/s390/include/asm/syscall.h
> index b3dd883699e7..1c0e349fd5c9 100644
> --- a/arch/s390/include/asm/syscall.h
> +++ b/arch/s390/include/asm/syscall.h
> @@ -24,6 +24,13 @@ static inline long syscall_get_nr(struct task_struct *task,
> (regs->int_code & 0x) : -1;
> }
>
> +static inline void syscall_set_nr(struct task_struct *task,
> + struct pt_regs *regs,
> + int nr)
> +{
I think there should be a
if (!test_pt_regs_flags(regs, PIF_SYSCALL))
return;
before the modification so a user can't accidentally change int_code
when ptrace stopped in a non-syscall path.
> + regs->int_code = (regs->int_code & ~0x) | (nr & 0x);
> +}
> +
> static inline void syscall_rollback(struct task_struct *task,
> struct pt_regs *regs)
> {
[PATCH bpf-next] bpf: veristat: Document verifier log dumping capability
`-l2 -v` is a useful combination of flags to dump the entire
verification log. This is helpful when making changes to the verifier,
as you can see what it thinks program one instruction at a time.
This was more or less a hidden feature before. Document it so others can
discover it.
Signed-off-by: Daniel Xu
---
tools/testing/selftests/bpf/veristat.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/tools/testing/selftests/bpf/veristat.c
b/tools/testing/selftests/bpf/veristat.c
index 974c808f9321..7d0a9cb753e3 100644
--- a/tools/testing/selftests/bpf/veristat.c
+++ b/tools/testing/selftests/bpf/veristat.c
@@ -216,7 +216,8 @@ const char argp_program_doc[] =
"\n"
"USAGE: veristat [...]\n"
" OR: veristat -C \n"
-" OR: veristat -R \n";
+" OR: veristat -R \n"
+" OR: veristat -v -l2 \n";
enum {
OPT_LOG_FIXED = 1000,
@@ -228,7 +229,7 @@ static const struct argp_option opts[] = {
{ "version", 'V', NULL, 0, "Print version" },
{ "verbose", 'v', NULL, 0, "Verbose mode" },
{ "debug", 'd', NULL, 0, "Debug mode (turns on libbpf debug logging)" },
- { "log-level", 'l', "LEVEL", 0, "Verifier log level (default 0 for
normal mode, 1 for verbose mode)" },
+ { "log-level", 'l', "LEVEL", 0, "Verifier log level (default 0 for
normal mode, 1 for verbose mode, 2 for full verification log)" },
{ "log-fixed", OPT_LOG_FIXED, NULL, 0, "Disable verifier log rotation"
},
{ "log-size", OPT_LOG_SIZE, "BYTES", 0, "Customize verifier log size
(default to 16MB)" },
{ "top-n", 'n', "N", 0, "Emit only up to first N results." },
--
2.47.1
Re: [sos-linux-ext-patches] [PATCH v4 1/8] KVM: SEV: Disable SEV-SNP on FW validation failure
On 11/15/2024 5:10 AM, Pratik R. Sampat wrote: > On incompatible firmware versions, SEV-SNP support is pulled and the > setup is not performed. However, the platform and subsequently the KVM > capability may continue to advertize support for it. Disable support for > SEV-SNP if the FW version validation fails. Additionally, can we ensure that if sev_platform_init() fails, we do not indicate SNP support? Regards Nikunj
Re: [PATCH 01/16] selftests/mm: remove argc and argv unused parameters
On Thu, 9 Jan 2025 09:50:45 -0800 Kees Cook wrote: > On Thu, Jan 09, 2025 at 10:48:52PM +0500, Muhammad Usama Anjum wrote: > > For the all other case, why should we keep argv/argc and mark them unused > > as well when they aren't being used? > > I'm fine either way, but my personal code style instinct is to keep the > "standard" main declaration with argc/argv present. But it's mostly > aesthetic. > > And if you think use of kselftest.h isn't universal, then perhaps we can > avoid the macro, but it does seem nicer and more "normal" feeling for > the rest of kernel development. > Agreed. __attribute__((unused)) is a bit of a mouthful and isn't what the kernel developer's eye expects to see.
Re: [PATCH] selftests/net/forwarding: teamd command not found
On Fri, Jan 10, 2025 at 01:07:44AM +0100, Alessandro Zanni wrote:
> Running "make kselftest TARGETS=net/forwarding" results in several
> occurrences of the same error:
> ./lib.sh: line 787: teamd: command not found
>
> Since many tests depends on teamd, this fix stops the tests if the
> teamd command is not installed.
>
> Signed-off-by: Alessandro Zanni
> ---
> tools/testing/selftests/net/forwarding/lib.sh | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/tools/testing/selftests/net/forwarding/lib.sh
> b/tools/testing/selftests/net/forwarding/lib.sh
> index 7337f398f9cc..a6a74a4be4bf 100644
> --- a/tools/testing/selftests/net/forwarding/lib.sh
> +++ b/tools/testing/selftests/net/forwarding/lib.sh
> @@ -784,6 +784,7 @@ team_destroy()
> {
> local if_name=$1; shift
>
> + require_command $TEAMD
> $TEAMD -t $if_name -k
> }
I saw team_create() has `require_command $TEAMD`. Is some test called
team_destroy() before team_create()?
Re: [PATCH] perf tools: Fixup end address of modules
On Wed, Dec 18, 2024 at 02:15:35PM -0800, Ian Rogers wrote: > On Wed, Dec 18, 2024 at 2:04 PM Namhyung Kim wrote: > > > > In machine__create_module(), it reads /proc/modules to get a list of > > modules in the system. The file shows the start address (of text) and > > the size of the module so it uses the info to reconstruct system memory > > maps for symbol resolution. > > > > But module memory consists of multiple segments and they can be > > scaterred. Currently perf tools assume they are contiguous and see some > > nit: s/scaterred/scattered/ > > > overlaps. This can confuse the tool when it finds a map containing a > > given address. > > > > As we mostly care about the function symbols in the text segment, it can > > fixup the size or end address of modules when there's an overlap. We > > can use maps__fixup_end() which updates the end address using the start > > address of the next map. > > > > Ideally it should be able to track other segments (like data/rodata), > > but that would require some changes in /proc/modules IMHO. > > > > Reported-by: Blake Jones > > Cc: Luis Chamberlain > > Cc: Petr Pavlu > > Cc: Sami Tolvanen > > Cc: Daniel Gomez > > Cc: linux-modu...@vger.kernel.org > > Signed-off-by: Namhyung Kim > > Acked-by: Ian Rogers Thanks, applied to perf-tools-next, - Arnaldo
Re: [RFC PATCH v1 1/2] mm/memfd: Add support for F_SEAL_FUTURE_EXEC to memfd
On Wed, Jan 8, 2025 at 11:06 AM Lorenzo Stoakes
wrote:
>
> On Mon, Jan 06, 2025 at 04:44:33PM -0800, Kees Cook wrote:
> > On Mon, Jan 06, 2025 at 10:26:27AM -0800, Jeff Xu wrote:
> > > + Kees because this is related to W^X memfd and security.
> > >
> > > On Fri, Jan 3, 2025 at 7:14 AM Jann Horn wrote:
> > > >
> > > > On Fri, Dec 6, 2024 at 7:19 PM Lorenzo Stoakes
> > > > wrote:
> > > > > On Thu, Dec 05, 2024 at 05:09:22PM -0800, Isaac J. Manjarres wrote:
> > > > > > + if (is_exec_sealed(seals)) {
> > > > >
> > > > > Are we intentionally disallowing a MAP_PRIVATE memfd's mapping's
> > > > > execution?
> > > > > I've not tested this scenario so don't know if we somehow disallow
> > > > > this in
> > > > > another way but note on write checks we only care about shared
> > > > > mappings.
> > > > >
> > > > > I mean one could argue that a MAP_PRIVATE situation is the same as
> > > > > copying
> > > > > the data into an anon buffer and doing what you want with it, here you
> > > > > could argue the same...
> > > > >
> > > > > So probably we should only care about VM_SHARED?
> > > >
> > > > FWIW I think it doesn't make sense to distinguish between
> > > > shared/private mappings here - in the scenario described in the cover
> > > > letter, it wouldn't matter that much to an attacker whether the
> > > > mapping is shared or private (as long as the VMA contents haven't been
> > > > CoWed already).
> > > +1 on this.
> > > The concept of blocking this for only shared mapping is questionable.
> >
> > Right -- why does sharedness matter? It seems more robust to me to not
> > create a corner case but rather apply the flag/behavior universally?
> >
>
> I'm struggling to understand what you are protecting against, if I can
> receive a
> buffer '-not executable-'. But then copy it into another buffer I mapped, and
> execute it?
>
preventing mmap() a memfd has the same threat model as preventing
execve() of a memfd, using execve() of a memfd as an example (since
the kernel already supports this): an attacker wanting to execute a
hijacked memfd must already have the ability to call execve() (e.g.,
by modifying a function pointer or using ROP). To prevent this, the
kernel supports making memfds non-executable (rw-) and permanently
preventing them from becoming executable (sealing with F_SEAL_EXEC).
Once the execve() attack path is blocked, the next thing an attacker
could do is mmap() the memfd into the process's memory and jump to it.
That is the reason I think we could tie this feature with
non-executable-bit + F_SEAL_EXEC, and avoid a new flag. This approach
leverages the existing memfd_create(MFD_NOEXEC_SEAL) function and
related sysctl for system level enforcement. This makes it simple for
applications to use the same function and ensures that both execve()
and mmap() are blocked by non-executable memfd. There is a small
backward compatibility risk for this approach, e.g. an application
already uses MFD_NOEXEC_SEAL but chooses to mmap it as executable, but
I think such a user case is strange to support.
If we're okay with using F_SEAL_FUTURE_EXEC, then share/private don't
matter, as the threat model explains. This flag's generic naming also
suggests it could apply to regular files in future. However, if this
flag is intended solely for shared memfd, it should be renamed to
something like F_SEAL_SHARED_MEMFD_FUTURE_EXEC_MAPPING. I don't think
this is the intention, right ?
LSM such as selinux is another option to consider, maybe it is a
better place to implement this? because selinux policy provides
system level control and enforcement, which the current implementation
lacks. If we end up having a selinux policy for this later, wouldn't
that obsolete this feature ?
-Jeff
[PATCH] selftests/net/forwarding: teamd command not found
Running "make kselftest TARGETS=net/forwarding" results in several
occurrences of the same error:
./lib.sh: line 787: teamd: command not found
Since many tests depends on teamd, this fix stops the tests if the
teamd command is not installed.
Signed-off-by: Alessandro Zanni
---
tools/testing/selftests/net/forwarding/lib.sh | 1 +
1 file changed, 1 insertion(+)
diff --git a/tools/testing/selftests/net/forwarding/lib.sh
b/tools/testing/selftests/net/forwarding/lib.sh
index 7337f398f9cc..a6a74a4be4bf 100644
--- a/tools/testing/selftests/net/forwarding/lib.sh
+++ b/tools/testing/selftests/net/forwarding/lib.sh
@@ -784,6 +784,7 @@ team_destroy()
{
local if_name=$1; shift
+ require_command $TEAMD
$TEAMD -t $if_name -k
}
--
2.43.0
Re: [PATCH 0/2] selftest: fix riscv/vector tests
On Fri, Dec 20, 2024 at 05:17:25PM +0800, Yong-Xuan Wang wrote: > Add test counts and pass message to remove warning of riscv/vector tests. A similar patch has been staged on Palmers for-next tree that I sent out a long time ago. I also reworked these tests so that they use the kselftest_harness [1]. [1] https://git.kernel.org/pub/scm/linux/kernel/git/palmer/linux.git/commit/?h=for-next&id=8fcaf922831887e32c8bdf5ebff76bbd80c971cb - Charlie > > Yong-Xuan Wang (2): > tools: selftests: riscv: Add pass message for v_initval_nolibc > tools: selftests: riscv: Add test count for vstate_prctl > > tools/testing/selftests/riscv/vector/v_initval_nolibc.c | 4 > tools/testing/selftests/riscv/vector/vstate_prctl.c | 2 ++ > 2 files changed, 6 insertions(+) > > -- > 2.17.1 > > > ___ > linux-riscv mailing list > linux-ri...@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-riscv
Re: [PATCH] virtio_ring: skip cpu sync when mapping fails
On Wed, Jan 8, 2025 at 7:35 PM Michael S. Tsirkin wrote: > > On Mon, Nov 11, 2024 at 10:55:38AM +0800, Jason Wang wrote: > > There's no need to sync DMA for CPU on mapping errors. So this patch > > skips the CPU sync in the error handling path of DMA mapping. > > > > Signed-off-by: Jason Wang > > > So as I said, I do not get why we are optimizing error paths. > The commit log at least needs to be improved to document > the motivation. As replied before. Does the following make more sense? 1) dma_map_sg() did this 2) When the driver tries to submit more buffers than SWIOTLB allows, dma_map might fail, in these cases, bouncing is useless. Thanks
Re: [PATCH v4 2/2] riscv: selftests: Add a ptrace test to verify syscall parameter modification
On Thu, Dec 26, 2024 at 11:21:59PM +0800, Celeste Liu wrote:
>
> On 2024-12-26 21:35, Dmitry V. Levin wrote:
> > On Thu, Dec 26, 2024 at 06:52:52PM +0800, Celeste Liu wrote:
> >> This test checks that orig_a0 allows a syscall argument to be modified,
> >> and that changing a0 does not change the syscall argument.
> >>
> >> Co-developed-by: Quan Zhou
> >> Signed-off-by: Quan Zhou
> >> Co-developed-by: Charlie Jenkins
> >> Signed-off-by: Charlie Jenkins
> >> Reviewed-by: Björn Töpel
> >> Signed-off-by: Celeste Liu
> > [...]
> >> diff --git a/tools/testing/selftests/riscv/abi/ptrace.c
> >> b/tools/testing/selftests/riscv/abi/ptrace.c
> >> new file mode 100644
> >> index
> >> ..023695352215bb5de3f91c1a6f5ea3b4f9373ff9
> >> --- /dev/null
> >> +++ b/tools/testing/selftests/riscv/abi/ptrace.c
> > [...]
> >> + if (ptrace(PTRACE_GET_SYSCALL_INFO, pid, PTRACE_SYSCALL_INFO_ENTRY,
> >> &syscall_info_entry))
> >> + perr_and_exit("failed to get syscall info of entry\n");
> >> + result->orig_a0 = syscall_info_entry->entry.args[0];
> >> + if (ptrace(PTRACE_GET_SYSCALL_INFO, pid, PTRACE_SYSCALL_INFO_EXIT,
> >> &syscall_info_exit))
> >> + perr_and_exit("failed to get syscall info of exit\n");
> >> + result->a0 = syscall_info_exit->exit.rval;
> >
> > I'm sorry but this is not how PTRACE_GET_SYSCALL_INFO should be used.
> >
> > PTRACE_GET_SYSCALL_INFO operation takes a pointer and a size,
> > and in this example instead of size you pass constants 1 and 2, which
> > essentially means that both syscall_info_entry->entry.args[0] and
> > syscall_info_exit->exit.rval are not going to be assigned
> > and would just contain some garbage from the stack.
> >
> > Also, PTRACE_GET_SYSCALL_INFO operation returns the number of bytes
> > available to be written by the kernel, which is always nonzero on any
> > PTRACE_GET_SYSCALL_INFO-capable kernel. In other words, this example
> > will always end up with perr_and_exit() call.
> >
> > I wonder how this test was tested before the submission.
>
> Oops... It seems I forget sync the code to test board so it runs with the old
> code...
> The code is completely not tested...
> I'm so sorry for my mistake.
>
> I will correct it and test it carefully later...
It would be great to get this into 6.14. Let me know if you would like
any help!
- Charlie
>
> >
> >
>
Re: [PATCH bpf-next v6 4/5] bpf: verifier: Support eliding map lookup nullness
On Thu, Jan 02, 2025 at 06:53:54PM -0800, Eduard Zingerman wrote:
> On Thu, 2024-12-19 at 21:09 -0700, Daniel Xu wrote:
>
> lgtm, but please see a note below.
>
> [...]
>
> > +/* Returns constant key value if possible, else negative error */
> > +static s64 get_constant_map_key(struct bpf_verifier_env *env,
> > + struct bpf_reg_state *key,
> > + u32 key_size)
> > +{
> > + struct bpf_func_state *state = func(env, key);
> > + struct bpf_reg_state *reg;
> > + int slot, spi, off;
> > + int spill_size = 0;
> > + int zero_size = 0;
> > + int stack_off;
> > + int i, err;
> > + u8 *stype;
> > +
> > + if (!env->bpf_capable)
> > + return -EOPNOTSUPP;
> > + if (key->type != PTR_TO_STACK)
> > + return -EOPNOTSUPP;
> > + if (!tnum_is_const(key->var_off))
> > + return -EOPNOTSUPP;
> > +
> > + stack_off = key->off + key->var_off.value;
> > + slot = -stack_off - 1;
> > + spi = slot / BPF_REG_SIZE;
> > + off = slot % BPF_REG_SIZE;
> > + stype = state->stack[spi].slot_type;
> > +
> > + /* First handle precisely tracked STACK_ZERO */
> > + for (i = off; i >= 0 && stype[i] == STACK_ZERO; i--)
> > + zero_size++;
> > + if (zero_size >= key_size)
> > + return 0;
> > +
> > + /* Check that stack contains a scalar spill of expected size */
> > + if (!is_spilled_scalar_reg(&state->stack[spi]))
> > + return -EOPNOTSUPP;
> > + for (i = off; i >= 0 && stype[i] == STACK_SPILL; i--)
> > + spill_size++;
> > + if (spill_size != key_size)
> > + return -EOPNOTSUPP;
> > +
> > + reg = &state->stack[spi].spilled_ptr;
> > + if (!tnum_is_const(reg->var_off))
> > + /* Stack value not statically known */
> > + return -EOPNOTSUPP;
> > +
> > + /* We are relying on a constant value. So mark as precise
> > +* to prevent pruning on it.
> > +*/
> > + bt_set_frame_slot(&env->bt, env->cur_state->curframe, spi);
>
> I think env->cur_state->curframe is not always correct here.
> It should be key->frameno, as key might point a few stack frames up.
Ack, nice catch.
Re: [PATCH] rseq/selftests: Fix riscv rseq_offset_deref_addv inline asm
On Fri, Jan 03, 2025 at 04:03:26AM +, Stafford Horne wrote:
> When working on OpenRISC support for restartable sequences I noticed
> and fixed these two issues with the riscv support bits.
>
> 1 The 'inc' argument to RSEQ_ASM_OP_R_DEREF_ADDV was being implicitly
>passed to the macro. Fix this by adding 'inc' to the list of macro
>arguments.
> 2 The inline asm input constraints for 'inc' and 'off' use "er", The
>riscv gcc port does not have an "e" constraint, this looks to be
>copied from the x86 port. Fix this by just using an "r" constraint.
>
> I have compile tested this only for riscv. However, the same fixes I
> use in the OpenRISC rseq selftests and everything passes with no issues.
Thank you for these changes! I suppose these tests hadn't been ran on
riscv before... I ran the tests on QEMU and they all passed :)
Tested-by: Charlie Jenkins
Reviewed-by: Charlie Jenkins
This should also have a fixes tag:
Fixes: 171586a6ab66 ("selftests/rseq: riscv: Template memory ordering and
percpu access mode")
>
> Signed-off-by: Stafford Horne
> ---
> tools/testing/selftests/rseq/rseq-riscv-bits.h | 6 +++---
> tools/testing/selftests/rseq/rseq-riscv.h | 2 +-
> 2 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/tools/testing/selftests/rseq/rseq-riscv-bits.h
> b/tools/testing/selftests/rseq/rseq-riscv-bits.h
> index de31a0143139..f02f411d550d 100644
> --- a/tools/testing/selftests/rseq/rseq-riscv-bits.h
> +++ b/tools/testing/selftests/rseq/rseq-riscv-bits.h
> @@ -243,7 +243,7 @@ int
> RSEQ_TEMPLATE_IDENTIFIER(rseq_offset_deref_addv)(intptr_t *ptr, off_t off, i
> #ifdef RSEQ_COMPARE_TWICE
> RSEQ_ASM_CMP_CPU_ID(cpu_id, current_cpu_id,
> "%l[error1]")
> #endif
> - RSEQ_ASM_OP_R_DEREF_ADDV(ptr, off, 3)
> + RSEQ_ASM_OP_R_DEREF_ADDV(ptr, off, inc, 3)
> RSEQ_INJECT_ASM(4)
> RSEQ_ASM_DEFINE_ABORT(4, abort)
> : /* gcc asm goto does not allow outputs */
> @@ -251,8 +251,8 @@ int
> RSEQ_TEMPLATE_IDENTIFIER(rseq_offset_deref_addv)(intptr_t *ptr, off_t off, i
> [current_cpu_id]"m"
> (rseq_get_abi()->RSEQ_TEMPLATE_CPU_ID_FIELD),
> [rseq_cs] "m"
> (rseq_get_abi()->rseq_cs.arch.ptr),
> [ptr] "r" (ptr),
> - [off] "er" (off),
> - [inc] "er" (inc)
> + [off] "r" (off),
> + [inc] "r" (inc)
> RSEQ_INJECT_INPUT
> : "memory", RSEQ_ASM_TMP_REG_1
> RSEQ_INJECT_CLOBBER
> diff --git a/tools/testing/selftests/rseq/rseq-riscv.h
> b/tools/testing/selftests/rseq/rseq-riscv.h
> index 37e598d0a365..67d544aaa9a3 100644
> --- a/tools/testing/selftests/rseq/rseq-riscv.h
> +++ b/tools/testing/selftests/rseq/rseq-riscv.h
> @@ -158,7 +158,7 @@ do {
> \
> "bnez " RSEQ_ASM_TMP_REG_1 ", 222b\n" \
> "333:\n"
>
> -#define RSEQ_ASM_OP_R_DEREF_ADDV(ptr, off, post_commit_label)
> \
> +#define RSEQ_ASM_OP_R_DEREF_ADDV(ptr, off, inc, post_commit_label) \
> "mv " RSEQ_ASM_TMP_REG_1 ", %[" __rseq_str(ptr) "]\n" \
> RSEQ_ASM_OP_R_ADD(off) \
> REG_L RSEQ_ASM_TMP_REG_1 ", 0(" RSEQ_ASM_TMP_REG_1 ")\n"\
> --
> 2.47.0
>
>
> ___
> linux-riscv mailing list
> linux-ri...@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv
Re: [PATCH net-next v3 0/4] netconsole: selftest for userdata overflow
Hello: This series was applied to netdev/net-next.git (main) by Jakub Kicinski : On Wed, 08 Jan 2025 03:50:24 -0800 you wrote: > Implement comprehensive testing for netconsole userdata entry handling, > demonstrating correct behavior when creating maximum entries and > preventing unauthorized overflow. > > Refactor existing test infrastructure to support modular, reusable > helper functions that validate strict entry limit enforcement. > > [...] Here is the summary with links: - [net-next,v3,1/4] netconsole: Warn if MAX_USERDATA_ITEMS limit is exceeded https://git.kernel.org/netdev/net-next/c/e51c7478d23b - [net-next,v3,2/4] netconsole: selftest: Split the helpers from the selftest https://git.kernel.org/netdev/net-next/c/61f51cc6defe - [net-next,v3,3/4] netconsole: selftest: Delete all userdata keys https://git.kernel.org/netdev/net-next/c/7dcb65351b30 - [net-next,v3,4/4] netconsole: selftest: verify userdata entry limit https://git.kernel.org/netdev/net-next/c/daea6d23cd2f You are awesome, thank you! -- Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/patchwork/pwbot.html
Re: [sos-linux-ext-patches] [PATCH v4 2/8] KVM: selftests: SEV-SNP test for KVM_SEV_INIT2
On 1/10/2025 11:22 AM, Nikunj A. Dadhania wrote:
>
>
> On 11/15/2024 5:10 AM, Pratik R. Sampat wrote:
>> Add the X86_FEATURE_SNP CPU feature to the architectural definition for
>> the SEV-SNP VM type to exercise the KVM_SEV_INIT2 call. Ensure that the
>> SNP test is skipped in scenarios where CPUID supports it but KVM does
>> not, so that a failure is not reported in such cases.
>>
>> Signed-off-by: Pratik R. Sampat
>> ---
>> .../selftests/kvm/include/x86_64/processor.h| 1 +
>> .../testing/selftests/kvm/x86_64/sev_init2_tests.c | 13 +
>> 2 files changed, 14 insertions(+)
>>
>> diff --git a/tools/testing/selftests/kvm/include/x86_64/processor.h
>> b/tools/testing/selftests/kvm/include/x86_64/processor.h
>> index 645200e95f89..c18d2405798f 100644
>> --- a/tools/testing/selftests/kvm/include/x86_64/processor.h
>> +++ b/tools/testing/selftests/kvm/include/x86_64/processor.h
>> @@ -199,6 +199,7 @@ struct kvm_x86_cpu_feature {
>> #define X86_FEATURE_VGIFKVM_X86_CPU_FEATURE(0x800A,
>> 0, EDX, 16)
>> #define X86_FEATURE_SEV KVM_X86_CPU_FEATURE(0x801F,
>> 0, EAX, 1)
>
> Unrelated to your patch, X86_FEATURE_SEV does not match the KVM API
> definitions,
> 1 is used for KVM_X86_SW_PROTECTED_VM
Scratch that, I got confused between the KVM API definition and CPUID naming.
Regards
Nikunj
[PATCH v4 3/4] rust: str: add radix prefixed integer parsing functions
Add the trait `ParseInt` for parsing string representations of integers
where the string representations are optionally prefixed by a radix
specifier. Implement the trait for the primitive integer types.
Signed-off-by: Andreas Hindborg
---
rust/kernel/str.rs | 118 +
1 file changed, 118 insertions(+)
diff --git a/rust/kernel/str.rs b/rust/kernel/str.rs
index
9c446ff1ad7adba7ca09a5ae9df00fd369a32899..14da40213f9eafa07a104eba3129efe07c8343f3
100644
--- a/rust/kernel/str.rs
+++ b/rust/kernel/str.rs
@@ -914,3 +914,121 @@ fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
macro_rules! fmt {
($($f:tt)*) => ( core::format_args!($($f)*) )
}
+
+pub mod parse_int {
+//! Integer parsing functions for parsing signed and unsigned integers
+//! potentially prefixed with `0x`, `0o`, or `0b`.
+
+use crate::alloc::flags;
+use crate::prelude::*;
+use crate::str::BStr;
+
+/// Trait that allows parsing a [`&BStr`] to an integer with a radix.
+///
+/// [`&BStr`]: kernel::str::BStr
+// This is required because the `from_str_radix` function on the primitive
+// integer types is not part of any trait.
+pub trait FromStrRadix: Sized {
+/// Parse `src` to `Self` using radix `radix`.
+fn from_str_radix(src: &BStr, radix: u32) -> Result;
+}
+
+/// Extract the radix from an integer literal optionally prefixed with
+/// one of `0x`, `0X`, `0o`, `0O`, `0b`, `0B`, `0`.
+fn strip_radix(src: &BStr) -> (u32, &BStr) {
+if let Some(n) = src.strip_prefix(b_str!("0x")) {
+(16, n)
+} else if let Some(n) = src.strip_prefix(b_str!("0X")) {
+(16, n)
+} else if let Some(n) = src.strip_prefix(b_str!("0o")) {
+(8, n)
+} else if let Some(n) = src.strip_prefix(b_str!("0O")) {
+(8, n)
+} else if let Some(n) = src.strip_prefix(b_str!("0b")) {
+(2, n)
+} else if let Some(n) = src.strip_prefix(b_str!("0B")) {
+(2, n)
+} else if let Some(n) = src.strip_prefix(b_str!("0")) {
+(8, n)
+} else {
+(10, src)
+}
+}
+
+/// Trait for parsing string representations of integers.
+///
+/// Strings beginning with `0x`, `0o`, or `0b` are parsed as hex, octal, or
+/// binary respectively. Strings beginning with `0` otherwise are parsed as
+/// octal. Anything else is parsed as decimal. A leading `+` or `-` is also
+/// permitted. Any string parsed by [`kstrtol()`] or [`kstrtoul()`] will be
+/// successfully parsed.
+///
+/// [`kstrtol()`]:
https://www.kernel.org/doc/html/latest/core-api/kernel-api.html#c.kstrtol
+/// [`kstrtoul()`]:
https://www.kernel.org/doc/html/latest/core-api/kernel-api.html#c.kstrtoul
+///
+/// # Example
+/// ```
+/// use kernel::str::parse_int::ParseInt;
+/// use kernel::b_str;
+///
+/// assert_eq!(Ok(0xa2u8), u8::from_str(b_str!("0xa2")));
+/// assert_eq!(Ok(-0xa2i32), i32::from_str(b_str!("-0xa2")));
+///
+/// assert_eq!(Ok(-0o57i8), i8::from_str(b_str!("-0o57")));
+/// assert_eq!(Ok(0o57i8), i8::from_str(b_str!("057")));
+///
+/// assert_eq!(Ok(0b1001i16), i16::from_str(b_str!("0b1001")));
+/// assert_eq!(Ok(-0b1001i16), i16::from_str(b_str!("-0b1001")));
+///
+/// assert_eq!(Ok(127), i8::from_str(b_str!("127")));
+/// assert!(i8::from_str(b_str!("128")).is_err());
+/// assert_eq!(Ok(-128), i8::from_str(b_str!("-128")));
+/// assert!(i8::from_str(b_str!("-129")).is_err());
+/// assert_eq!(Ok(255), u8::from_str(b_str!("255")));
+/// assert!(u8::from_str(b_str!("256")).is_err());
+/// ```
+pub trait ParseInt: FromStrRadix {
+/// Parse a string according to the description in [`Self`].
+fn from_str(src: &BStr) -> Result {
+match src.iter().next() {
+None => Err(EINVAL),
+Some(sign @ b'-') | Some(sign @ b'+') => {
+let (radix, digits) =
strip_radix(BStr::from_bytes(&src[1..]));
+let mut n_digits: KVec =
+KVec::with_capacity(digits.len() + 1,
flags::GFP_KERNEL)?;
+n_digits.push(*sign, flags::GFP_KERNEL)?;
+n_digits.extend_from_slice(digits, flags::GFP_KERNEL)?;
+Self::from_str_radix(BStr::from_bytes(&n_digits),
radix).map_err(|_| EINVAL)
+}
+Some(_) => {
+let (radix, digits) = strip_radix(src);
+Self::from_str_radix(digits, radix).map_err(|_| EINVAL)
+}
+}
+}
+}
+
+macro_rules! impl_parse_int {
+($ty:ty) => {
+impl FromStrRadix for $ty {
+fn from_str_radix(src: &BStr, radix: u32) -> Result {
+
<$ty>::from_str_radix(core::str::from_utf8(src).map_err(|_| E
Re: [PATCH RFC 2/2] module: Introduce hash-based integrity checking
On Fri, 3 Jan 2025 17:37:52 -0800, Luis Chamberlain wrote: > What distro which is using module signatures would switch > to this as an alternative instead? In NixOS, we disable MODULE_SIG by default (because we value reproducibility over having module signatures). Enabling MODULE_HASHES on systems that do not need to load out-of-tree modules would be a good step forward. Kind regards, Arnout Engelen
[PATCH v4 4/4] rust: add parameter support to the `module!` macro
This patch includes changes required for Rust kernel modules to utilize
module parameters. This code implements read only support for integer
types without `sysfs` support.
Signed-off-by: Andreas Hindborg
---
rust/kernel/lib.rs | 1 +
rust/kernel/module_param.rs | 225 +++
rust/macros/helpers.rs | 14 +++
rust/macros/lib.rs | 31 ++
rust/macros/module.rs| 188
samples/rust/rust_minimal.rs | 10 ++
6 files changed, 451 insertions(+), 18 deletions(-)
diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
index
4253c9a7fe7df935dc714bc3036d299bd80054a0..707ec81411c7aecccdd14e02cbf011642bc02e07
100644
--- a/rust/kernel/lib.rs
+++ b/rust/kernel/lib.rs
@@ -46,6 +46,7 @@
pub mod kunit;
pub mod list;
pub mod miscdevice;
+pub mod module_param;
#[cfg(CONFIG_NET)]
pub mod net;
pub mod once_lite;
diff --git a/rust/kernel/module_param.rs b/rust/kernel/module_param.rs
new file mode 100644
index
..6cb64096090a37c4f9c4ce73e23b7ac049e32d20
--- /dev/null
+++ b/rust/kernel/module_param.rs
@@ -0,0 +1,225 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! Types for module parameters.
+//!
+//! C header:
[`include/linux/moduleparam.h`](srctree/include/linux/moduleparam.h)
+
+use crate::prelude::*;
+use crate::str::BStr;
+
+/// Newtype to make `bindings::kernel_param` [`Sync`].
+#[repr(transparent)]
+#[doc(hidden)]
+pub struct RacyKernelParam(pub ::kernel::bindings::kernel_param);
+
+// SAFETY: C kernel handles serializing access to this type. We never access
+// from Rust module.
+unsafe impl Sync for RacyKernelParam {}
+
+/// Types that can be used for module parameters.
+///
+/// Note that displaying the type in `sysfs` will fail if
+/// [`Display`](core::fmt::Display) implementation would write more than
+/// [`PAGE_SIZE`] - 1 bytes.
+///
+/// [`PAGE_SIZE`]: `bindings::PAGE_SIZE`
+pub trait ModuleParam: Sized {
+/// The [`ModuleParam`] will be used by the kernel module through this
type.
+///
+/// This may differ from `Self` if, for example, `Self` needs to track
+/// ownership without exposing it or allocate extra space for other
possible
+/// parameter values.
+// This is required to support string parameters in the future.
+type Value: ?Sized;
+
+/// Parse a parameter argument into the parameter value.
+///
+/// `Err(_)` should be returned when parsing of the argument fails.
+///
+/// Parameters passed at boot time will be set before [`kmalloc`] is
+/// available (even if the module is loaded at a later time). However, in
+/// this case, the argument buffer will be valid for the entire lifetime of
+/// the kernel. So implementations of this method which need to allocate
+/// should first check that the allocator is available (with
+/// [`crate::bindings::slab_is_available`]) and when it is not available
+/// provide an alternative implementation which doesn't allocate. In cases
+/// where the allocator is not available it is safe to save references to
+/// `arg` in `Self`, but in other cases a copy should be made.
+///
+/// [`kmalloc`]: srctree/include/linux/slab.h
+fn try_from_param_arg(arg: &'static [u8]) -> Result;
+}
+
+/// Set the module parameter from a string.
+///
+/// Used to set the parameter value at kernel initialization, when loading
+/// the module or when set through `sysfs`.
+///
+/// `param.arg` is a pointer to `*mut T` as set up by the [`module!`]
+/// macro.
+///
+/// See `struct kernel_param_ops.set`.
+///
+/// # Safety
+///
+/// If `val` is non-null then it must point to a valid null-terminated
+/// string. The `arg` field of `param` must be an instance of `T`.
+///
+/// # Invariants
+///
+/// Currently, we only support read-only parameters that are not readable
+/// from `sysfs`. Thus, this function is only called at kernel
+/// initialization time, or at module load time, and we have exclusive
+/// access to the parameter for the duration of the function.
+///
+/// [`module!`]: macros::module
+unsafe extern "C" fn set_param(
+val: *const core::ffi::c_char,
+param: *const crate::bindings::kernel_param,
+) -> core::ffi::c_int
+where
+T: ModuleParam,
+{
+// NOTE: If we start supporting arguments without values, val _is_ allowed
+// to be null here.
+if val.is_null() {
+crate::pr_warn_once!("Null pointer passed to
`module_param::set_param`");
+return crate::error::code::EINVAL.to_errno();
+}
+
+// SAFETY: By function safety requirement, val is non-null and
+// null-terminated. By C API contract, `val` is live and valid for reads
+// for the duration of this function.
+let arg = unsafe { CStr::from_char_ptr(val).as_bytes() };
+
+crate::error::from_result(|| {
+let new_value = T::try_from_param_arg(arg)?;
+
+// SAFETY: `param` is guaranteed to be valid by C API contrac
[PATCH v4 0/4] rust: extend `module!` macro with integer parameter support
This series extends the `module!` macro with support module parameters. It
also adds some string to integer parsing functions and updates `BStr` with
a method to strip a string prefix.
This series stated out as code by Adam Bratschi-Kaye lifted from the original
`rust` branch [1].
After a bit of discussion on v3 about whether or not module parameters
is a good idea, it seems that module parameters in Rust has a place
in the kernel for now. This series is a dependency for `rnull`, the Rust
null block driver [2].
Link:
https://github.com/Rust-for-Linux/linux/tree/bc22545f38d74473cfef3e9fd65432733435b79f
[1]
Link:
https://git.kernel.org/pub/scm/linux/kernel/git/a.hindborg/linux.git/log/?h=rnull
[2]
Signed-off-by: Andreas Hindborg
---
Changes in v4:
- Add module maintainers to Cc list (sorry)
- Add a few missing [`doc_links`]
- Add panic section to `expect_string_field`
- Fix a typo in safety requirement of `module_params::free`
- Change `assert!` to `pr_warn_once!` in `module_params::set_param`
- Remove `module_params::get_param` and install null pointer instead
- Remove use of the unstable feature `sync_unsafe_cell`
- Link to v3:
https://lore.kernel.org/r/20241213-module-params-v3-v3-0-485a015ac...@kernel.org
Changes in v3:
- use `SyncUnsafeCell` rather than `static mut` and simplify parameter access
- remove `Display` bound from `ModuleParam`
- automatically generate documentation for `PARAM_OPS_.*`
- remove `as *const _ as *mut_` phrasing
- inline parameter name in struct instantiation in `emit_params`
- move `RacyKernelParam` out of macro template
- use C string literals rather than byte string literals with explicit null
- template out `__{name}_{param_name}` in `emit_param`
- indent template in `emit_params`
- use let-else expression in `emit_params` to get rid of an indentation level
- document `expect_string_field`
- move invication of `impl_int_module_param` to be closer to macro def
- move attributes after docs in `make_param_ops`
- rename `impl_module_param` to impl_int_module_param`
- use `ty` instead of `ident` in `impl_parse_int`
- use `BStr` instead of `&str` for string manipulation
- move string parsing functions to seperate patch and add examples, fix bugs
- degrade comment about future support from doc comment to regular comment
- remove std lib path from `Sized` marker
- update documentation for `trait ModuleParam`
- Link to v2:
https://lore.kernel.org/all/20240819133345.3438739-1-...@metaspace.dk/
Changes in v2:
- Remove support for params without values (`NOARG_ALLOWED`).
- Improve documentation for `try_from_param_arg`.
- Use prelude import.
- Refactor `try_from_param_arg` to return `Result`.
- Refactor `ParseInt::from_str` to return `Result`.
- Move C callable functions out of `ModuleParam` trait.
- Rename literal string field parser to `expect_string_field`.
- Move parameter parsing from generation to parsing stage.
- Use absolute type paths in macro code.
- Inline `kparam`and `read_func` values.
- Resolve TODO regarding alignment attributes.
- Remove unnecessary unsafe blocks in macro code.
- Improve error message for unrecognized parameter types.
- Do not use `self` receiver when reading parameter value.
- Add parameter documentation to `module!` macro.
- Use empty `enum` for parameter type.
- Use `addr_of_mut` to get address of parameter value variable.
- Enabled building of docs for for `module_param` module.
- Link to v1:
https://lore.kernel.org/rust-for-linux/20240705111455.142790-1-...@metaspace.dk/
---
Andreas Hindborg (4):
rust: str: implement `PartialEq` for `BStr`
rust: str: implement `strip_prefix` for `BStr`
rust: str: add radix prefixed integer parsing functions
rust: add parameter support to the `module!` macro
rust/kernel/lib.rs | 1 +
rust/kernel/module_param.rs | 225 +++
rust/kernel/str.rs | 140 +++
rust/macros/helpers.rs | 14 +++
rust/macros/lib.rs | 31 ++
rust/macros/module.rs| 188
samples/rust/rust_minimal.rs | 10 ++
7 files changed, 591 insertions(+), 18 deletions(-)
---
base-commit: e32a80927434907f973f38a88cd19d7e51991d24
change-id: 20241211-module-params-v3-ae7e5c8d8b5a
prerequisite-change-id: 20241107-pr_once_macros-6438e6f5b923:v4
prerequisite-patch-id: 57743fff5d9c649ff7c1aed7e374d08ae67dda91
prerequisite-patch-id: fe607e3e1f666e7250bf099e581d53c83fea5f7d
prerequisite-patch-id: eb030eccf23466b0e22e7c699f252c40bd5f21bf
Best regards,
--
Andreas Hindborg
[PATCH v4 1/4] rust: str: implement `PartialEq` for `BStr`
Implement `PartialEq` for `BStr` by comparing underlying byte slices.
Reviewed-by: Alice Ryhl
Signed-off-by: Andreas Hindborg
---
rust/kernel/str.rs | 6 ++
1 file changed, 6 insertions(+)
diff --git a/rust/kernel/str.rs b/rust/kernel/str.rs
index
d04c12a1426d1c1edeb88325bcd9c63bf45f9b60..c441acf76ebd1f14919b6d233edfff944619
100644
--- a/rust/kernel/str.rs
+++ b/rust/kernel/str.rs
@@ -106,6 +106,12 @@ fn deref(&self) -> &Self::Target {
}
}
+impl PartialEq for BStr {
+fn eq(&self, other: &Self) -> bool {
+self.deref().eq(other.deref())
+}
+}
+
/// Creates a new [`BStr`] from a string literal.
///
/// `b_str!` converts the supplied string literal to byte string, so non-ASCII
--
2.47.0
Re: [PATCH net 1/2] vsock/virtio: discard packets if the transport changes
On Thu, Jan 09, 2025 at 04:13:44AM -0500, Hyunwoo Kim wrote:
On Thu, Jan 09, 2025 at 10:01:31AM +0100, Stefano Garzarella wrote:
On Wed, Jan 08, 2025 at 02:31:19PM -0500, Hyunwoo Kim wrote:
> On Wed, Jan 08, 2025 at 07:06:16PM +0100, Stefano Garzarella wrote:
> > If the socket has been de-assigned or assigned to another transport,
> > we must discard any packets received because they are not expected
> > and would cause issues when we access vsk->transport.
> >
> > A possible scenario is described by Hyunwoo Kim in the attached link,
> > where after a first connect() interrupted by a signal, and a second
> > connect() failed, we can find `vsk->transport` at NULL, leading to a
> > NULL pointer dereference.
> >
> > Fixes: c0cfa2d8a788 ("vsock: add multi-transports support")
> > Reported-by: Hyunwoo Kim
> > Reported-by: Wongi Lee
> > Closes:
https://lore.kernel.org/netdev/Z2LvdTTQR7dBmPb5@v4bel-B760M-AORUS-ELITE-AX/
> > Signed-off-by: Stefano Garzarella
> > ---
> > net/vmw_vsock/virtio_transport_common.c | 7 +--
> > 1 file changed, 5 insertions(+), 2 deletions(-)
> >
> > diff --git a/net/vmw_vsock/virtio_transport_common.c
b/net/vmw_vsock/virtio_transport_common.c
> > index 9acc13ab3f82..51a494b69be8 100644
> > --- a/net/vmw_vsock/virtio_transport_common.c
> > +++ b/net/vmw_vsock/virtio_transport_common.c
> > @@ -1628,8 +1628,11 @@ void virtio_transport_recv_pkt(struct
virtio_transport *t,
> >
> > lock_sock(sk);
> >
> > - /* Check if sk has been closed before lock_sock */
> > - if (sock_flag(sk, SOCK_DONE)) {
> > + /* Check if sk has been closed or assigned to another transport before
> > + * lock_sock (note: listener sockets are not assigned to any
transport)
> > + */
> > + if (sock_flag(sk, SOCK_DONE) ||
> > + (sk->sk_state != TCP_LISTEN && vsk->transport != &t->transport)) {
>
> If a race scenario with vsock_listen() is added to the existing
> race scenario, the patch can be bypassed.
>
> In addition to the existing scenario:
> ```
> cpu0
cpu1
>
> socket(A)
>
> bind(A, {cid:
VMADDR_CID_LOCAL, port: 1024})
> vsock_bind()
>
> listen(A)
> vsock_listen()
> socket(B)
>
> connect(B, {cid: VMADDR_CID_LOCAL, port: 1024})
>vsock_connect()
> lock_sock(sk);
> virtio_transport_connect()
>virtio_transport_connect()
> virtio_transport_send_pkt_info()
>vsock_loopback_send_pkt(VIRTIO_VSOCK_OP_REQUEST)
> queue_work(vsock_loopback_work)
> sk->sk_state = TCP_SYN_SENT;
> release_sock(sk);
>
vsock_loopback_work()
>
virtio_transport_recv_pkt(VIRTIO_VSOCK_OP_REQUEST)
> sk =
vsock_find_bound_socket(&dst);
>
virtio_transport_recv_listen(sk, skb)
> child =
vsock_create_connected(sk);
>
vsock_assign_transport()
> vvs =
kzalloc(sizeof(*vvs), GFP_KERNEL);
>
vsock_insert_connected(vchild);
>
list_add(&vsk->connected_table, list);
>
virtio_transport_send_response(vchild, skb);
>
virtio_transport_send_pkt_info()
>
vsock_loopback_send_pkt(VIRTIO_VSOCK_OP_RESPONSE)
>
queue_work(vsock_loopback_work)
>
>
vsock_loopback_work()
>
virtio_transport_recv_pkt(VIRTIO_VSOCK_OP_RESPONSE)
> sk =
vsock_find_bound_socket(&dst);
>
lock_sock(sk);
> case
TCP_SYN_SENT:
>
virtio_transport_recv_connecting()
>
sk->sk_st
Re: [PATCH V3 6/8] arm64: dts: qcom: ipq5332: add nodes to bringup q6
On 7.01.2025 11:16 AM, Gokul Sriram Palanisamy wrote:
> From: Manikanta Mylavarapu
>
> Enable nodes required for q6 remoteproc bring up.
>
> Signed-off-by: Manikanta Mylavarapu
> Signed-off-by: Gokul Sriram Palanisamy
> ---
[...]
> + q6v5_wcss: remoteproc@d10 {
> + compatible = "qcom,ipq5332-wcss-sec-pil";
> + reg = <0xd10 0x4040>;
Please pad the address part to 8 hex digits with leading zeroes
> + firmware-name = "ath12k/IPQ5332/hw1.0/q6_fw0.mdt";
> + interrupts-extended = <&intc GIC_SPI 421
> IRQ_TYPE_EDGE_RISING>,
> + <&wcss_smp2p_in 0 IRQ_TYPE_NONE>,
> + <&wcss_smp2p_in 1 IRQ_TYPE_NONE>,
> + <&wcss_smp2p_in 2 IRQ_TYPE_NONE>,
> + <&wcss_smp2p_in 3 IRQ_TYPE_NONE>;
> + interrupt-names = "wdog",
> + "fatal",
> + "ready",
> + "handover",
> + "stop-ack";
> +
> + clocks = <&gcc GCC_IM_SLEEP_CLK>;
> + clock-names = "sleep";
> +
> + qcom,smem-states = <&wcss_smp2p_out 1>,
> +<&wcss_smp2p_out 0>;
> + qcom,smem-state-names = "stop",
> + "shutdown";
> +
> + memory-region = <&q6_region>;
> +
> + glink-edge {
> + interrupts = ;
> + label = "rtr";
> + qcom,remote-pid = <1>;
> + mboxes = <&apcs_glb 8>;
> + };
> + };
> };
>
> timer {
> @@ -488,4 +526,28 @@ timer {
> IRQ_TYPE_LEVEL_LOW)>,
> IRQ_TYPE_LEVEL_LOW)>;
> };
> +
> + wcss: wcss-smp2p {
All other DTs (except the odd cookie ipq6018 which sneaked through) call
these smp2p-foo instead of foo-smp2p
Konrad
Re: [PATCH V3 7/8] arm64: dts: qcom: ipq9574: add nodes to bring up q6
On 7.01.2025 11:16 AM, Gokul Sriram Palanisamy wrote: > From: Manikanta Mylavarapu > > Enable nodes required for q6 remoteproc bring up. > > Signed-off-by: Manikanta Mylavarapu > Signed-off-by: Gokul Sriram Palanisamy > --- The comments you got on patch 6 apply here and to patch 8 too Konrad
Re: [PATCH 1/3] selftests/mm: virtual_address_range: Fix error when CommitLimit < 1GiB
On 09.01.25 14:05, David Hildenbrand wrote: > That is clear. The issue would be to figure which chunks are valid to unmap. If something critical like the executable file is unmapped, the process crashes. But see below. Ah, now I see what you mean. Yes, also the stack etc. will be problematic. So IIUC, you want to limit the munmap optimization only to the manually mmap()ed parts. Is it fine to rely on CONFIG_ANON_VMA_NAME? That would make it much easier to implement. Can you elaborate how you would do it? First set the VMA name after mmap(): I took a look at the implementation, and VMA merging seems to be able to merge such VMAs that share the same name (even when set separately). So assuming you use the same name for all, that should indeed also work. -- Cheers, David / dhildenb
[PATCH 02/16] selftests/mm: Fix unused parameter warnings
Fix warnings generated by unused parameters.
Signed-off-by: Muhammad Usama Anjum
---
.../selftests/mm/hugetlb_fault_after_madv.c | 4 +-
.../selftests/mm/hugetlb_madv_vs_map.c| 6 +--
tools/testing/selftests/mm/ksm_tests.c| 17 +++---
tools/testing/selftests/mm/memfd_secret.c | 4 +-
tools/testing/selftests/mm/soft-dirty.c | 4 +-
.../selftests/mm/split_huge_page_test.c | 4 +-
tools/testing/selftests/mm/uffd-common.c | 18 ---
tools/testing/selftests/mm/uffd-common.h | 2 +-
tools/testing/selftests/mm/uffd-stress.c | 2 +-
tools/testing/selftests/mm/uffd-unit-tests.c | 52 +--
10 files changed, 57 insertions(+), 56 deletions(-)
diff --git a/tools/testing/selftests/mm/hugetlb_fault_after_madv.c
b/tools/testing/selftests/mm/hugetlb_fault_after_madv.c
index e2640529dbb29..e62f4e1388f26 100644
--- a/tools/testing/selftests/mm/hugetlb_fault_after_madv.c
+++ b/tools/testing/selftests/mm/hugetlb_fault_after_madv.c
@@ -28,7 +28,7 @@ static void signal_handler(int signal)
}
/* Touch the memory while it is being madvised() */
-void *touch(void *unused)
+void *touch(void __attribute__((unused)) *unused)
{
char *ptr = (char *)huge_ptr;
@@ -41,7 +41,7 @@ void *touch(void *unused)
return NULL;
}
-void *madv(void *unused)
+void *madv(void __attribute__((unused)) *unused)
{
usleep(rand() % 10);
diff --git a/tools/testing/selftests/mm/hugetlb_madv_vs_map.c
b/tools/testing/selftests/mm/hugetlb_madv_vs_map.c
index 8f122a0f08281..6c326cf3dcf6b 100644
--- a/tools/testing/selftests/mm/hugetlb_madv_vs_map.c
+++ b/tools/testing/selftests/mm/hugetlb_madv_vs_map.c
@@ -33,7 +33,7 @@ size_t mmap_size;
char *huge_ptr;
/* Touch the memory while it is being madvised() */
-void *touch(void *unused)
+void *touch(void __attribute__((unused)) *unused)
{
for (int i = 0; i < INLOOP_ITER; i++)
huge_ptr[0] = '.';
@@ -41,7 +41,7 @@ void *touch(void *unused)
return NULL;
}
-void *madv(void *unused)
+void *madv(void __attribute__((unused)) *unused)
{
for (int i = 0; i < INLOOP_ITER; i++)
madvise(huge_ptr, mmap_size, MADV_DONTNEED);
@@ -54,7 +54,7 @@ void *madv(void *unused)
* The other hugepage should be flipping from used <-> reserved, because
* of madvise(DONTNEED).
*/
-void *map_extra(void *unused)
+void *map_extra(void __attribute__((unused)) *unused)
{
void *ptr;
diff --git a/tools/testing/selftests/mm/ksm_tests.c
b/tools/testing/selftests/mm/ksm_tests.c
index dcdd5bb20f3d8..323cfcb14e4d4 100644
--- a/tools/testing/selftests/mm/ksm_tests.c
+++ b/tools/testing/selftests/mm/ksm_tests.c
@@ -265,8 +265,7 @@ static int ksm_merge_pages(int merge_type, void *addr,
size_t size,
return 0;
}
-static int ksm_unmerge_pages(void *addr, size_t size,
-struct timespec start_time, int timeout)
+static int ksm_unmerge_pages(void *addr, size_t size)
{
if (madvise(addr, size, MADV_UNMERGEABLE)) {
perror("madvise");
@@ -483,7 +482,7 @@ static int get_first_mem_node(void)
return get_next_mem_node(numa_max_node());
}
-static int check_ksm_numa_merge(int merge_type, int mapping, int prot, int
timeout,
+static int check_ksm_numa_merge(int merge_type, int timeout,
bool merge_across_nodes, size_t page_size)
{
void *numa1_map_ptr, *numa2_map_ptr;
@@ -547,8 +546,7 @@ static int check_ksm_numa_merge(int merge_type, int
mapping, int prot, int timeo
return KSFT_FAIL;
}
-static int ksm_merge_hugepages_time(int merge_type, int mapping, int prot,
- int timeout, size_t map_size)
+static int ksm_merge_hugepages_time(int merge_type, int timeout, size_t
map_size)
{
void *map_ptr, *map_ptr_orig;
struct timespec start_time, end_time;
@@ -678,7 +676,7 @@ static int ksm_unmerge_time(int merge_type, int mapping,
int prot, int timeout,
perror("clock_gettime");
goto err_out;
}
- if (ksm_unmerge_pages(map_ptr, map_size, start_time, timeout))
+ if (ksm_unmerge_pages(map_ptr, map_size))
goto err_out;
if (clock_gettime(CLOCK_MONOTONIC_RAW, &end_time)) {
perror("clock_gettime");
@@ -906,8 +904,8 @@ int main(int argc, char *argv[])
page_size);
break;
case CHECK_KSM_NUMA_MERGE:
- ret = check_ksm_numa_merge(merge_type, MAP_PRIVATE |
MAP_ANONYMOUS, prot,
- ksm_scan_limit_sec, merge_across_nodes,
page_size);
+ ret = check_ksm_numa_merge(merge_type, ksm_scan_limit_sec,
merge_across_nodes,
+ page_size);
break;
case KSM_MERGE_TIME:
if (size_MB == 0) {
@@ -922,8 +920,7 @@ int main(int argc, char *argv[])
[PATCH 01/16] selftests/mm: remove argc and argv unused parameters
Remove the following warnings by removing unused argc and argv
parameters:
In function ‘main’:
warning: unused parameter ‘argc’ [-Wunused-parameter]
158 | int main(int argc, char *argv[])
| ^~~~
warning: unused parameter ‘argv’ [-Wunused-parameter]
158 | int main(int argc, char *argv[])
|~~^~
Signed-off-by: Muhammad Usama Anjum
---
tools/testing/selftests/mm/compaction_test.c | 2 +-
tools/testing/selftests/mm/cow.c | 2 +-
tools/testing/selftests/mm/droppable.c | 2 +-
tools/testing/selftests/mm/gup_longterm.c | 2 +-
tools/testing/selftests/mm/hugepage-vmemmap.c | 2 +-
tools/testing/selftests/mm/hugetlb-madvise.c | 2 +-
tools/testing/selftests/mm/hugetlb-soft-offline.c | 2 +-
tools/testing/selftests/mm/madv_populate.c | 2 +-
tools/testing/selftests/mm/map_populate.c | 2 +-
tools/testing/selftests/mm/memfd_secret.c | 2 +-
tools/testing/selftests/mm/mlock-random-test.c | 2 +-
tools/testing/selftests/mm/mlock2-tests.c | 2 +-
tools/testing/selftests/mm/on-fault-limit.c| 2 +-
tools/testing/selftests/mm/pkey_sighandler_tests.c | 2 +-
tools/testing/selftests/mm/soft-dirty.c| 2 +-
tools/testing/selftests/mm/uffd-wp-mremap.c| 2 +-
tools/testing/selftests/mm/virtual_address_range.c | 2 +-
17 files changed, 17 insertions(+), 17 deletions(-)
diff --git a/tools/testing/selftests/mm/compaction_test.c
b/tools/testing/selftests/mm/compaction_test.c
index 2c3a0eb6b22d3..8d23b698ce9db 100644
--- a/tools/testing/selftests/mm/compaction_test.c
+++ b/tools/testing/selftests/mm/compaction_test.c
@@ -194,7 +194,7 @@ int set_zero_hugepages(unsigned long *initial_nr_hugepages)
return ret;
}
-int main(int argc, char **argv)
+int main(void)
{
struct rlimit lim;
struct map_list *list = NULL, *entry;
diff --git a/tools/testing/selftests/mm/cow.c b/tools/testing/selftests/mm/cow.c
index 1238e1c5aae15..ea00c85c76caa 100644
--- a/tools/testing/selftests/mm/cow.c
+++ b/tools/testing/selftests/mm/cow.c
@@ -1769,7 +1769,7 @@ static int tests_per_non_anon_test_case(void)
return tests;
}
-int main(int argc, char **argv)
+int main(void)
{
int err;
struct thp_settings default_settings;
diff --git a/tools/testing/selftests/mm/droppable.c
b/tools/testing/selftests/mm/droppable.c
index f3d9ecf96890a..90ea6377810c5 100644
--- a/tools/testing/selftests/mm/droppable.c
+++ b/tools/testing/selftests/mm/droppable.c
@@ -15,7 +15,7 @@
#include "../kselftest.h"
-int main(int argc, char *argv[])
+int main(void)
{
size_t alloc_size = 134217728;
size_t page_size = getpagesize();
diff --git a/tools/testing/selftests/mm/gup_longterm.c
b/tools/testing/selftests/mm/gup_longterm.c
index 9423ad439a614..03a31dcb57577 100644
--- a/tools/testing/selftests/mm/gup_longterm.c
+++ b/tools/testing/selftests/mm/gup_longterm.c
@@ -444,7 +444,7 @@ static int tests_per_test_case(void)
return 3 + nr_hugetlbsizes;
}
-int main(int argc, char **argv)
+int main(void)
{
int i, err;
diff --git a/tools/testing/selftests/mm/hugepage-vmemmap.c
b/tools/testing/selftests/mm/hugepage-vmemmap.c
index df366a4d1b92d..23e97e552057d 100644
--- a/tools/testing/selftests/mm/hugepage-vmemmap.c
+++ b/tools/testing/selftests/mm/hugepage-vmemmap.c
@@ -87,7 +87,7 @@ static int check_page_flags(unsigned long pfn)
return 0;
}
-int main(int argc, char **argv)
+int main(void)
{
void *addr;
unsigned long pfn;
diff --git a/tools/testing/selftests/mm/hugetlb-madvise.c
b/tools/testing/selftests/mm/hugetlb-madvise.c
index e74107185324f..43f16c12c8e9a 100644
--- a/tools/testing/selftests/mm/hugetlb-madvise.c
+++ b/tools/testing/selftests/mm/hugetlb-madvise.c
@@ -58,7 +58,7 @@ void read_fault_pages(void *addr, unsigned long nr_pages)
}
}
-int main(int argc, char **argv)
+int main(int __attribute__((unused)) argc, char **argv)
{
unsigned long free_hugepages;
void *addr, *addr2;
diff --git a/tools/testing/selftests/mm/hugetlb-soft-offline.c
b/tools/testing/selftests/mm/hugetlb-soft-offline.c
index f086f0e04756f..cb087303f5ed3 100644
--- a/tools/testing/selftests/mm/hugetlb-soft-offline.c
+++ b/tools/testing/selftests/mm/hugetlb-soft-offline.c
@@ -216,7 +216,7 @@ static void test_soft_offline_common(int
enable_soft_offline)
enable_soft_offline);
}
-int main(int argc, char **argv)
+int main(void)
{
ksft_print_header();
ksft_set_plan(2);
diff --git a/tools/testing/selftests/mm/madv_populate.c
b/tools/testing/selftests/mm/madv_populate.c
index ef7d911da13e0..c6a3ee56a54a9 100644
--- a/tools/testing/selftests/mm/madv_populate.c
+++ b/tools/testing/selftests/mm/madv_populate.c
@@ -281,7 +281,7 @@ static int system_has_softdirty(void)
#endif
}
-int main(int argc, char **argv)
+int main(void)
{
i
[PATCH 00/16] selftest/mm: Remove warnings found by adding compiler flags
Recently, I reviewed a patch on the mm/kselftest mailing list about a test which had obvious type mismatch fix in it. It was strange why that wasn't caught during development and when patch was accepted. This led me to discover that those extra compiler options to catch these warnings aren't being used. When I added them, I found tens of warnings in just mm suite. In this series, I'm fixing those warnings. The last check adds the compiler flags with which the warnings have been caught. Muhammad Usama Anjum (16): selftests/mm: remove argc and argv unused parameters selftests/mm: Fix unused parameter warnings selftests/mm: Fix unused parameter warnings selftests/mm: Fix type mismatch warnings selftests/mm: kselftest_harness: Fix warnings selftests/mm: cow: remove unused variables and fix type mismatch errors selftests/mm: hmm-tests: Remove always false expressions selftests/mm: guard-pages: Fix type mismatch warnings selftests/mm: hugetlb-madvise: fix type mismatch issues selftests/mm: hugepage-vmemmap: fix type mismatch warnings selftests/mm: hugetlb-read-hwpoison: Fix type mismatch warnings selftests/mm: khugepaged: Fix type mismatch warnings selftests/mm: protection_keys: Fix variables types mismatch warnings selftests/mm: thuge-gen: Fix type mismatch warnings selftests/mm: uffd-*: Fix all type mismatch warnings selftests/mm: Makefile: Add the compiler flags tools/testing/selftests/kselftest_harness.h | 6 +- tools/testing/selftests/mm/Makefile | 1 + tools/testing/selftests/mm/compaction_test.c | 4 +- tools/testing/selftests/mm/cow.c | 48 --- tools/testing/selftests/mm/droppable.c| 2 +- tools/testing/selftests/mm/guard-pages.c | 4 +- tools/testing/selftests/mm/gup_longterm.c | 5 +- tools/testing/selftests/mm/hmm-tests.c| 4 +- tools/testing/selftests/mm/hugepage-vmemmap.c | 5 +- tools/testing/selftests/mm/hugetlb-madvise.c | 4 +- .../selftests/mm/hugetlb-read-hwpoison.c | 6 +- .../selftests/mm/hugetlb-soft-offline.c | 2 +- tools/testing/selftests/mm/hugetlb_dio.c | 2 +- .../selftests/mm/hugetlb_fault_after_madv.c | 6 +- .../selftests/mm/hugetlb_madv_vs_map.c| 8 +-- tools/testing/selftests/mm/khugepaged.c | 16 ++--- .../selftests/mm/ksm_functional_tests.c | 6 +- tools/testing/selftests/mm/ksm_tests.c| 17 +++--- tools/testing/selftests/mm/madv_populate.c| 2 +- tools/testing/selftests/mm/map_populate.c | 2 +- tools/testing/selftests/mm/memfd_secret.c | 6 +- .../testing/selftests/mm/mlock-random-test.c | 6 +- tools/testing/selftests/mm/mlock2-tests.c | 2 +- tools/testing/selftests/mm/on-fault-limit.c | 2 +- tools/testing/selftests/mm/pkey-x86.h | 4 +- .../selftests/mm/pkey_sighandler_tests.c | 21 --- tools/testing/selftests/mm/protection_keys.c | 34 ++- tools/testing/selftests/mm/soft-dirty.c | 8 +-- .../selftests/mm/split_huge_page_test.c | 4 +- tools/testing/selftests/mm/thuge-gen.c| 10 ++-- tools/testing/selftests/mm/uffd-common.c | 24 tools/testing/selftests/mm/uffd-common.h | 2 +- tools/testing/selftests/mm/uffd-stress.c | 4 +- tools/testing/selftests/mm/uffd-unit-tests.c | 58 +-- tools/testing/selftests/mm/uffd-wp-mremap.c | 2 +- .../selftests/mm/virtual_address_range.c | 2 +- 36 files changed, 180 insertions(+), 159 deletions(-) -- 2.39.5
Re: [PATCH 01/16] selftests/mm: remove argc and argv unused parameters
On Thu, Jan 09, 2025 at 10:48:52PM +0500, Muhammad Usama Anjum wrote: > For the all other case, why should we keep argv/argc and mark them unused > as well when they aren't being used? I'm fine either way, but my personal code style instinct is to keep the "standard" main declaration with argc/argv present. But it's mostly aesthetic. And if you think use of kselftest.h isn't universal, then perhaps we can avoid the macro, but it does seem nicer and more "normal" feeling for the rest of kernel development. -Kees -- Kees Cook
[PATCH 05/16] selftests/mm: kselftest_harness: Fix warnings
Found warnings through hmm-tests and mdwe_test.
Fix following warnings:
- Mark unused variable with unused attribute
- __EXPECT is causing types mismatch warnings when __exp is unsigned and
_seen is equal to a constant number, __typeof__(_seen) returns signed
type.
hmm-tests.c: In function ‘hmm_anon_read’:
../kselftest_harness.h:523:52: warning: comparison of integer expressions of
different signedness: ‘long unsigned int’ and ‘int’ [-Wsign-compare]
523 | __EXPECT(expected, #expected, seen, #seen, !=, 1)
|^~
../kselftest_harness.h:759:21: note: in definition of macro ‘__EXPECT’
759 | if (!(__exp _t __seen)) { \
| ^~
hmm-tests.c:303:9: note: in expansion of macro ‘ASSERT_NE’
303 | ASSERT_NE(npages, 0);
| ^
- Mark variant as unused:
mdwe_test.c: In function ‘wrapper_prctl_flags’:
../kselftest_harness.h:177:52: warning: unused parameter ‘variant’
[-Wunused-parameter]
177 | struct __fixture_variant_metadata *variant) \
| ~~~^~~
../kselftest_harness.h:152:25: note: in expansion of macro ‘__TEST_IMPL’
152 | #define TEST(test_name) __TEST_IMPL(test_name, -1)
| ^~~
mdwe_test.c:23:1: note: in expansion of macro ‘TEST’
23 | TEST(prctl_flags)
| ^~~~
Signed-off-by: Muhammad Usama Anjum
---
tools/testing/selftests/kselftest_harness.h | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/tools/testing/selftests/kselftest_harness.h
b/tools/testing/selftests/kselftest_harness.h
index 666c9fde76da9..76e6b3be0e9d6 100644
--- a/tools/testing/selftests/kselftest_harness.h
+++ b/tools/testing/selftests/kselftest_harness.h
@@ -174,7 +174,7 @@
static void test_name(struct __test_metadata *_metadata); \
static inline void wrapper_##test_name( \
struct __test_metadata *_metadata, \
- struct __fixture_variant_metadata *variant) \
+ struct __fixture_variant_metadata __attribute__((unused))
*variant) \
{ \
_metadata->setup_completed = true; \
if (setjmp(_metadata->env) == 0) \
@@ -756,7 +756,7 @@
/* Avoid multiple evaluation of the cases */ \
__typeof__(_expected) __exp = (_expected); \
__typeof__(_seen) __seen = (_seen); \
- if (!(__exp _t __seen)) { \
+ if (!(__exp _t (__typeof__(_expected)) __seen)) { \
/* Report with actual signedness to avoid weird output. */ \
switch (is_signed_type(__exp) * 2 + is_signed_type(__seen)) { \
case 0: { \
@@ -965,7 +965,7 @@ static inline void __test_check_assert(struct
__test_metadata *t)
}
struct __test_metadata *__active_test;
-static void __timeout_handler(int sig, siginfo_t *info, void *ucontext)
+static void __timeout_handler(int sig, siginfo_t *info, void
__attribute__((unused)) *ucontext)
{
struct __test_metadata *t = __active_test;
--
2.39.5
[PATCH 04/16] selftests/mm: Fix type mismatch warnings
Fix type mismatch warnings in different tests.
Signed-off-by: Muhammad Usama Anjum
---
tools/testing/selftests/mm/compaction_test.c | 2 +-
tools/testing/selftests/mm/gup_longterm.c | 3 ++-
tools/testing/selftests/mm/hugetlb_dio.c | 2 +-
tools/testing/selftests/mm/hugetlb_fault_after_madv.c | 2 +-
tools/testing/selftests/mm/hugetlb_madv_vs_map.c | 2 +-
tools/testing/selftests/mm/ksm_functional_tests.c | 6 +++---
tools/testing/selftests/mm/mlock-random-test.c| 4 ++--
tools/testing/selftests/mm/pkey_sighandler_tests.c| 2 +-
tools/testing/selftests/mm/soft-dirty.c | 2 +-
9 files changed, 13 insertions(+), 12 deletions(-)
diff --git a/tools/testing/selftests/mm/compaction_test.c
b/tools/testing/selftests/mm/compaction_test.c
index 8d23b698ce9db..f6f32a5732e9e 100644
--- a/tools/testing/selftests/mm/compaction_test.c
+++ b/tools/testing/selftests/mm/compaction_test.c
@@ -134,7 +134,7 @@ int check_compaction(unsigned long mem_free, unsigned long
hugepage_size,
lseek(fd, 0, SEEK_SET);
if (write(fd, init_nr_hugepages, strlen(init_nr_hugepages))
- != strlen(init_nr_hugepages)) {
+ != (signed long int)strlen(init_nr_hugepages)) {
ksft_print_msg("Failed to write value to
/proc/sys/vm/nr_hugepages: %s\n",
strerror(errno));
goto close_fd;
diff --git a/tools/testing/selftests/mm/gup_longterm.c
b/tools/testing/selftests/mm/gup_longterm.c
index 03a31dcb57577..7f1b4ad7fcaec 100644
--- a/tools/testing/selftests/mm/gup_longterm.c
+++ b/tools/testing/selftests/mm/gup_longterm.c
@@ -446,7 +446,8 @@ static int tests_per_test_case(void)
int main(void)
{
- int i, err;
+ unsigned int i;
+ int err;
pagesize = getpagesize();
nr_hugetlbsizes = detect_hugetlb_page_sizes(hugetlbsizes,
diff --git a/tools/testing/selftests/mm/hugetlb_dio.c
b/tools/testing/selftests/mm/hugetlb_dio.c
index db63abe5ee5e8..62f368d4c8c16 100644
--- a/tools/testing/selftests/mm/hugetlb_dio.c
+++ b/tools/testing/selftests/mm/hugetlb_dio.c
@@ -63,7 +63,7 @@ void run_dio_using_hugetlb(unsigned int start_off, unsigned
int end_off)
memset(buffer, 'A', writesize);
/* Write the buffer to the file */
- if (write(fd, buffer, writesize) != (writesize)) {
+ if (write(fd, buffer, writesize) != (signed int)writesize) {
munmap(orig_buffer, h_pagesize);
close(fd);
ksft_exit_fail_perror("Error writing to file\n");
diff --git a/tools/testing/selftests/mm/hugetlb_fault_after_madv.c
b/tools/testing/selftests/mm/hugetlb_fault_after_madv.c
index e62f4e1388f26..2b5acb13ee0be 100644
--- a/tools/testing/selftests/mm/hugetlb_fault_after_madv.c
+++ b/tools/testing/selftests/mm/hugetlb_fault_after_madv.c
@@ -88,7 +88,7 @@ int main(void)
MAP_PRIVATE | MAP_ANONYMOUS | MAP_HUGETLB,
-1, 0);
- if ((unsigned long)huge_ptr == -1)
+ if (huge_ptr == MAP_FAILED)
ksft_exit_skip("Failed to allocated huge page\n");
pthread_create(&thread1, NULL, madv, NULL);
diff --git a/tools/testing/selftests/mm/hugetlb_madv_vs_map.c
b/tools/testing/selftests/mm/hugetlb_madv_vs_map.c
index 6c326cf3dcf6b..eda38b63e9e8d 100644
--- a/tools/testing/selftests/mm/hugetlb_madv_vs_map.c
+++ b/tools/testing/selftests/mm/hugetlb_madv_vs_map.c
@@ -100,7 +100,7 @@ int main(void)
MAP_PRIVATE | MAP_ANONYMOUS | MAP_HUGETLB,
-1, 0);
- if ((unsigned long)huge_ptr == -1) {
+ if (huge_ptr == MAP_FAILED) {
ksft_test_result_fail("Failed to allocate huge page\n");
return KSFT_FAIL;
}
diff --git a/tools/testing/selftests/mm/ksm_functional_tests.c
b/tools/testing/selftests/mm/ksm_functional_tests.c
index 66b4e111b5a27..4f96126e4e1f9 100644
--- a/tools/testing/selftests/mm/ksm_functional_tests.c
+++ b/tools/testing/selftests/mm/ksm_functional_tests.c
@@ -306,7 +306,7 @@ static void test_unmerge_zero_pages(void)
/* Check if ksm_zero_pages is updated correctly after KSM merging */
pages_expected = size / pagesize;
- if (pages_expected != get_my_ksm_zero_pages()) {
+ if ((signed long)pages_expected != get_my_ksm_zero_pages()) {
ksft_test_result_fail("'ksm_zero_pages' updated after
merging\n");
goto unmap;
}
@@ -319,7 +319,7 @@ static void test_unmerge_zero_pages(void)
/* Check if ksm_zero_pages is updated correctly after unmerging */
pages_expected /= 2;
- if (pages_expected != get_my_ksm_zero_pages()) {
+ if ((signed long)pages_expected != get_my_ksm_zero_pages()) {
ksft_test_result_fail("'ksm_zero_pages' updated after
unmerging\n");
[PATCH 03/16] selftests/mm: Fix unused parameter warnings
Fix unused parameter warnings.
Signed-off-by: Muhammad Usama Anjum
---
tools/testing/selftests/mm/pkey-x86.h | 4 +++-
.../selftests/mm/pkey_sighandler_tests.c | 17 +++--
tools/testing/selftests/mm/protection_keys.c | 18 ++
3 files changed, 24 insertions(+), 15 deletions(-)
diff --git a/tools/testing/selftests/mm/pkey-x86.h
b/tools/testing/selftests/mm/pkey-x86.h
index f7ecd335df1e6..75cef8ee00702 100644
--- a/tools/testing/selftests/mm/pkey-x86.h
+++ b/tools/testing/selftests/mm/pkey-x86.h
@@ -157,7 +157,9 @@ static inline void expect_fault_on_read_execonly_key(void
*p1, int pkey)
expected_pkey_fault(pkey);
}
-static inline void *malloc_pkey_with_mprotect_subpage(long size, int prot, u16
pkey)
+static inline void *malloc_pkey_with_mprotect_subpage(long
__attribute__((unused)) size,
+ int
__attribute__((unused)) prot,
+ u16
__attribute__((unused)) pkey)
{
return PTR_ERR_ENOTSUP;
}
diff --git a/tools/testing/selftests/mm/pkey_sighandler_tests.c
b/tools/testing/selftests/mm/pkey_sighandler_tests.c
index 249989f8b7a2a..cd46528d6c215 100644
--- a/tools/testing/selftests/mm/pkey_sighandler_tests.c
+++ b/tools/testing/selftests/mm/pkey_sighandler_tests.c
@@ -42,7 +42,8 @@ static siginfo_t siginfo = {0};
* which is protected by MPK 0 which we don't have access to.
*/
static inline __always_inline
-long syscall_raw(long n, long a1, long a2, long a3, long a4, long a5, long a6)
+long syscall_raw(long n, long a1, long a2, long a3, long a4, long a5,
+long __attribute__((unused)) a6)
{
unsigned long ret;
#ifdef __x86_64__
@@ -110,7 +111,8 @@ static inline u64 pkey_reg_restrictive_default(void)
return set_pkey_bits(PKEY_REG_ALLOW_NONE, 0, PKEY_DISABLE_ACCESS);
}
-static void sigsegv_handler(int signo, siginfo_t *info, void *ucontext)
+static void sigsegv_handler(int __attribute__((unused)) signo, siginfo_t *info,
+ void __attribute__((unused)) *ucontext)
{
pthread_mutex_lock(&mutex);
@@ -122,7 +124,8 @@ static void sigsegv_handler(int signo, siginfo_t *info,
void *ucontext)
syscall_raw(SYS_exit, 0, 0, 0, 0, 0, 0);
}
-static void sigusr1_handler(int signo, siginfo_t *info, void *ucontext)
+static void sigusr1_handler(int __attribute__((unused)) signo, siginfo_t *info,
+ void __attribute__((unused)) *ucontext)
{
pthread_mutex_lock(&mutex);
@@ -132,7 +135,9 @@ static void sigusr1_handler(int signo, siginfo_t *info,
void *ucontext)
pthread_mutex_unlock(&mutex);
}
-static void sigusr2_handler(int signo, siginfo_t *info, void *ucontext)
+static void sigusr2_handler(int __attribute__((unused)) signo,
+ siginfo_t __attribute__((unused)) *info,
+ void __attribute__((unused)) *ucontext)
{
/*
* pkru should be the init_pkru value which enabled MPK 0 so
@@ -155,7 +160,7 @@ static void raise_sigusr2(void)
*/
}
-static void *thread_segv_with_pkey0_disabled(void *ptr)
+static void *thread_segv_with_pkey0_disabled(void __attribute__((unused)) *ptr)
{
/* Disable MPK 0 (and all others too) */
__write_pkey_reg(pkey_reg_restrictive_default());
@@ -165,7 +170,7 @@ static void *thread_segv_with_pkey0_disabled(void *ptr)
return NULL;
}
-static void *thread_segv_pkuerr_stack(void *ptr)
+static void *thread_segv_pkuerr_stack(void __attribute__((unused)) *ptr)
{
/* Disable MPK 0 (and all others too) */
__write_pkey_reg(pkey_reg_restrictive_default());
diff --git a/tools/testing/selftests/mm/protection_keys.c
b/tools/testing/selftests/mm/protection_keys.c
index a4683f2476f27..3a66798a8e8d6 100644
--- a/tools/testing/selftests/mm/protection_keys.c
+++ b/tools/testing/selftests/mm/protection_keys.c
@@ -309,7 +309,7 @@ static char *si_code_str(int si_code)
static int pkey_faults;
static int last_si_pkey = -1;
-static void signal_handler(int signum, siginfo_t *si, void *vucontext)
+static void signal_handler(int __attribute__((unused)) signum, siginfo_t *si,
void *vucontext)
{
ucontext_t *uctxt = vucontext;
int trapno;
@@ -891,7 +891,7 @@ static void close_test_fds(void)
nr_test_fds = 0;
}
-static void test_pkey_alloc_free_attach_pkey0(int *ptr, u16 pkey)
+static void test_pkey_alloc_free_attach_pkey0(int *ptr, u16
__attribute__((unused)) pkey)
{
int i, err;
int max_nr_pkey_allocs;
@@ -1108,7 +1108,7 @@ static void test_pkey_syscalls_on_non_allocated_pkey(int
*ptr, u16 pkey)
}
/* Assumes that all pkeys other than 'pkey' are unallocated */
-static void test_pkey_syscalls_bad_args(int *ptr, u16 pkey)
+static void test_pkey_syscalls_bad_args(int *ptr, u16 __attribute__((unused))
pkey)
{
int err;
int bad_pkey = NR_PKEYS+99;
@@ -1134,7 +113
[PATCH 11/16] selftests/mm: hugetlb-read-hwpoison: Fix type mismatch warnings
Fix type mismatch warnings.
Signed-off-by: Muhammad Usama Anjum
---
tools/testing/selftests/mm/hugetlb-read-hwpoison.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/tools/testing/selftests/mm/hugetlb-read-hwpoison.c
b/tools/testing/selftests/mm/hugetlb-read-hwpoison.c
index ba6cc6f9cabcd..e2a2bb1989d58 100644
--- a/tools/testing/selftests/mm/hugetlb-read-hwpoison.c
+++ b/tools/testing/selftests/mm/hugetlb-read-hwpoison.c
@@ -72,7 +72,7 @@ static bool seek_read_hugepage_filemap(int fd, size_t len,
size_t wr_chunk_size,
{
char buf[MAX_WRITE_READ_CHUNK_SIZE];
ssize_t ret_count = 0;
- ssize_t total_ret_count = 0;
+ size_t total_ret_count = 0;
char val = offset / wr_chunk_size + offset % wr_chunk_size;
printf(PREFIX PREFIX "init val=%u with offset=0x%lx\n", val, offset);
@@ -83,7 +83,7 @@ static bool seek_read_hugepage_filemap(int fd, size_t len,
size_t wr_chunk_size,
return false;
}
- while (offset + total_ret_count < len) {
+ while ((unsigned long)offset + total_ret_count < len) {
ret_count = read(fd, buf, wr_chunk_size);
if (ret_count == 0) {
printf(PREFIX PREFIX "read reach end of the file\n");
@@ -109,7 +109,7 @@ static bool read_hugepage_filemap(int fd, size_t len,
{
char buf[MAX_WRITE_READ_CHUNK_SIZE];
ssize_t ret_count = 0;
- ssize_t total_ret_count = 0;
+ size_t total_ret_count = 0;
char val = 0;
printf(PREFIX PREFIX "expect to read 0x%lx bytes of data in total\n",
--
2.39.5
[PATCH 06/16] selftests/mm: cow: remove unused variables and fix type mismatch errors
Fix following warnings:
- Remove unused variables
- Use __attribute__(unused) with unused variables which aren't being
used and cannot be removed because of function pointer declaration
- Fix type mismatches
Signed-off-by: Muhammad Usama Anjum
---
tools/testing/selftests/mm/cow.c | 46 +++-
1 file changed, 27 insertions(+), 19 deletions(-)
diff --git a/tools/testing/selftests/mm/cow.c b/tools/testing/selftests/mm/cow.c
index ea00c85c76caa..e37eb863e66cd 100644
--- a/tools/testing/selftests/mm/cow.c
+++ b/tools/testing/selftests/mm/cow.c
@@ -84,7 +84,7 @@ static void detect_huge_zeropage(void)
return;
ret = pread(fd, buf, sizeof(buf), 0);
- if (ret > 0 && ret < sizeof(buf)) {
+ if (ret > 0 && (unsigned int)ret < sizeof(buf)) {
buf[ret] = 0;
enabled = strtoul(buf, NULL, 10);
@@ -263,12 +263,14 @@ static void do_test_cow_in_parent(char *mem, size_t size,
bool do_mprotect,
close_comm_pipes(&comm_pipes);
}
-static void test_cow_in_parent(char *mem, size_t size, bool is_hugetlb)
+static void test_cow_in_parent(char *mem, size_t size,
+ bool __attribute__((unused)) is_hugetlb)
{
do_test_cow_in_parent(mem, size, false, child_memcmp_fn, false);
}
-static void test_cow_in_parent_mprotect(char *mem, size_t size, bool
is_hugetlb)
+static void test_cow_in_parent_mprotect(char *mem, size_t size,
+ bool __attribute__((unused)) is_hugetlb)
{
do_test_cow_in_parent(mem, size, true, child_memcmp_fn, false);
}
@@ -408,10 +410,11 @@ static void do_test_iouring(char *mem, size_t size, bool
use_fork)
struct io_uring_cqe *cqe;
struct io_uring_sqe *sqe;
struct io_uring ring;
- ssize_t cur, total;
struct iovec iov;
char *buf, *tmp;
+ size_t total;
int ret, fd;
+ ssize_t cur;
FILE *file;
ret = setup_comm_pipes(&comm_pipes);
@@ -515,7 +518,7 @@ static void do_test_iouring(char *mem, size_t size, bool
use_fork)
goto quit_child;
}
- if (cqe->res != size) {
+ if ((unsigned int) cqe->res != size) {
ksft_test_result_fail("write_fixed failed\n");
goto quit_child;
}
@@ -529,7 +532,7 @@ static void do_test_iouring(char *mem, size_t size, bool
use_fork)
ksft_test_result_fail("pread() failed\n");
goto quit_child;
}
- total += cur;
+ total += (size_t)cur;
}
/* Finally, check if we read what we expected. */
@@ -553,12 +556,14 @@ static void do_test_iouring(char *mem, size_t size, bool
use_fork)
close_comm_pipes(&comm_pipes);
}
-static void test_iouring_ro(char *mem, size_t size, bool is_hugetlb)
+static void test_iouring_ro(char *mem, size_t size,
+ bool __attribute__((unused)) is_hugetlb)
{
do_test_iouring(mem, size, false);
}
-static void test_iouring_fork(char *mem, size_t size, bool is_hugetlb)
+static void test_iouring_fork(char *mem, size_t size,
+ bool __attribute__((unused)) is_hugetlb)
{
do_test_iouring(mem, size, true);
}
@@ -702,36 +707,38 @@ static void do_test_ro_pin(char *mem, size_t size, enum
ro_pin_test test,
free(tmp);
}
-static void test_ro_pin_on_shared(char *mem, size_t size, bool is_hugetlb)
+static void test_ro_pin_on_shared(char *mem, size_t size,
+ bool __attribute__((unused)) is_hugetlb)
{
do_test_ro_pin(mem, size, RO_PIN_TEST_SHARED, false);
}
-static void test_ro_fast_pin_on_shared(char *mem, size_t size, bool is_hugetlb)
+static void test_ro_fast_pin_on_shared(char *mem, size_t size,
+ bool __attribute__((unused)) is_hugetlb)
{
do_test_ro_pin(mem, size, RO_PIN_TEST_SHARED, true);
}
static void test_ro_pin_on_ro_previously_shared(char *mem, size_t size,
- bool is_hugetlb)
+ bool __attribute__((unused))
is_hugetlb)
{
do_test_ro_pin(mem, size, RO_PIN_TEST_PREVIOUSLY_SHARED, false);
}
static void test_ro_fast_pin_on_ro_previously_shared(char *mem, size_t size,
- bool is_hugetlb)
+bool
__attribute__((unused)) is_hugetlb)
{
do_test_ro_pin(mem, size, RO_PIN_TEST_PREVIOUSLY_SHARED, true);
}
static void test_ro_pin_on_ro_exclusive(char *mem, size_t size,
- bool is_hugetlb)
+ bool __attribute__((unused)) is_hugetlb)
{
do_test_ro_pin(mem, size, RO_PIN_TEST_RO_EXCLUSIVE, false);
}
static void test_ro_fast_pin_on_ro_exclusive(char *mem, size_t size,
- bool is_hugetlb)
+bool __attribute__((unuse
[PATCH 07/16] selftests/mm: hmm-tests: Remove always false expressions
As default_hsize is unsigned, it is always greater than equal to zero.
Remove expression which is checking if it is less than 0.
Signed-off-by: Muhammad Usama Anjum
---
tools/testing/selftests/mm/hmm-tests.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tools/testing/selftests/mm/hmm-tests.c
b/tools/testing/selftests/mm/hmm-tests.c
index 141bf63cbe05e..3b4db583bd3b6 100644
--- a/tools/testing/selftests/mm/hmm-tests.c
+++ b/tools/testing/selftests/mm/hmm-tests.c
@@ -796,7 +796,7 @@ TEST_F(hmm, anon_write_hugetlbfs)
int ret;
default_hsize = file_read_ulong("/proc/meminfo", "Hugepagesize:");
- if (default_hsize < 0 || default_hsize*1024 < default_hsize)
+ if (default_hsize*1024 < default_hsize)
SKIP(return, "Huge page size could not be determined");
default_hsize = default_hsize*1024; /* KB to B */
@@ -1579,7 +1579,7 @@ TEST_F(hmm, compound)
/* Skip test if we can't allocate a hugetlbfs page. */
default_hsize = file_read_ulong("/proc/meminfo", "Hugepagesize:");
- if (default_hsize < 0 || default_hsize*1024 < default_hsize)
+ if (default_hsize*1024 < default_hsize)
SKIP(return, "Huge page size could not be determined");
default_hsize = default_hsize*1024; /* KB to B */
--
2.39.5
[PATCH 13/16] selftests/mm: protection_keys: Fix variables types mismatch warnings
Fix variable type mismatch warnings.
Signed-off-by: Muhammad Usama Anjum
---
tools/testing/selftests/mm/protection_keys.c | 16
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/tools/testing/selftests/mm/protection_keys.c
b/tools/testing/selftests/mm/protection_keys.c
index 3a66798a8e8d6..ab1e586bc2728 100644
--- a/tools/testing/selftests/mm/protection_keys.c
+++ b/tools/testing/selftests/mm/protection_keys.c
@@ -78,7 +78,7 @@ static void cat_into_file(char *str, char *file)
}
ret = write(fd, str, strlen(str));
- if (ret != strlen(str)) {
+ if (ret != (signed int)strlen(str)) {
perror("write to file failed");
fprintf(stderr, "filename: '%s' str: '%s'\n", file, str);
exit(__LINE__);
@@ -597,10 +597,10 @@ struct pkey_malloc_record *pkey_last_malloc_record;
static long nr_pkey_malloc_records;
void record_pkey_malloc(void *ptr, long size, int prot)
{
- long i;
+ unsigned long i;
struct pkey_malloc_record *rec = NULL;
- for (i = 0; i < nr_pkey_malloc_records; i++) {
+ for (i = 0; i < (unsigned long)nr_pkey_malloc_records; i++) {
rec = &pkey_malloc_records[i];
/* find a free record */
if (rec)
@@ -866,7 +866,7 @@ static int nr_test_fds;
static void __save_test_fd(int fd)
{
pkey_assert(fd >= 0);
- pkey_assert(nr_test_fds < ARRAY_SIZE(test_fds));
+ pkey_assert(nr_test_fds < (signed int)ARRAY_SIZE(test_fds));
test_fds[nr_test_fds] = fd;
nr_test_fds++;
}
@@ -897,7 +897,7 @@ static void test_pkey_alloc_free_attach_pkey0(int *ptr, u16
__attribute__((unuse
int max_nr_pkey_allocs;
int alloced_pkeys[NR_PKEYS];
int nr_alloced = 0;
- long size;
+ unsigned long size;
pkey_assert(pkey_last_malloc_record);
size = pkey_last_malloc_record->size;
@@ -1280,7 +1280,7 @@ static void test_pkey_init_state(int
__attribute__((unused)) *ptr, u16 __attribu
*/
static void test_mprotect_with_pkey_0(int *ptr, u16 pkey)
{
- long size;
+ unsigned long size;
int prot;
assert(pkey_last_malloc_record);
@@ -1530,7 +1530,7 @@ static void test_ptrace_modifies_pkru(int
__attribute__((unused)) *ptr,
pkey_assert(WIFSTOPPED(status) && WSTOPSIG(status) == SIGSTOP);
xsave = (void *)malloc(xsave_size);
- pkey_assert(xsave > 0);
+ pkey_assert(xsave != NULL);
/* Modify the PKRU register directly */
iov.iov_base = xsave;
@@ -1727,7 +1727,7 @@ static void run_tests_once(void)
int *ptr;
int prot = PROT_READ|PROT_WRITE;
- for (test_nr = 0; test_nr < ARRAY_SIZE(pkey_tests); test_nr++) {
+ for (test_nr = 0; test_nr < (signed int)ARRAY_SIZE(pkey_tests);
test_nr++) {
int pkey;
int orig_pkey_faults = pkey_faults;
--
2.39.5
[PATCH 08/16] selftests/mm: guard-pages: Fix type mismatch warnings
Fix type mismatch warnings. Signed-off-by: Muhammad Usama Anjum --- tools/testing/selftests/mm/guard-pages.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tools/testing/selftests/mm/guard-pages.c b/tools/testing/selftests/mm/guard-pages.c index ece37212a8a2e..fc1165ef2015f 100644 --- a/tools/testing/selftests/mm/guard-pages.c +++ b/tools/testing/selftests/mm/guard-pages.c @@ -142,7 +142,7 @@ TEST_F(guard_pages, basic) const unsigned long NUM_PAGES = 10; const unsigned long page_size = self->page_size; char *ptr; - int i; + unsigned int i; ptr = mmap(NULL, NUM_PAGES * page_size, PROT_READ | PROT_WRITE, MAP_PRIVATE | MAP_ANON, -1, 0); @@ -1044,7 +1044,7 @@ TEST_F(guard_pages, fork_cow) const unsigned long page_size = self->page_size; char *ptr; pid_t pid; - int i; + unsigned int i; /* Map 10 pages. */ ptr = mmap(NULL, 10 * page_size, PROT_READ | PROT_WRITE, -- 2.39.5
[PATCH 10/16] selftests/mm: hugepage-vmemmap: fix type mismatch warnings
Fix type mismatch warnings.
Signed-off-by: Muhammad Usama Anjum
---
tools/testing/selftests/mm/hugepage-vmemmap.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/mm/hugepage-vmemmap.c
b/tools/testing/selftests/mm/hugepage-vmemmap.c
index 23e97e552057d..8d30ebfc9b86c 100644
--- a/tools/testing/selftests/mm/hugepage-vmemmap.c
+++ b/tools/testing/selftests/mm/hugepage-vmemmap.c
@@ -51,7 +51,8 @@ static unsigned long virt_to_pfn(void *addr)
static int check_page_flags(unsigned long pfn)
{
- int fd, i;
+ int fd;
+ unsigned int i;
unsigned long pageflags;
fd = open("/proc/kpageflags", O_RDONLY);
--
2.39.5
[PATCH 16/16] selftests/mm: Makefile: Add the compiler flags
Add the compiler flags to catch warnings during development. Signed-off-by: Muhammad Usama Anjum --- tools/testing/selftests/mm/Makefile | 1 + 1 file changed, 1 insertion(+) diff --git a/tools/testing/selftests/mm/Makefile b/tools/testing/selftests/mm/Makefile index 63ce39d024bb5..006ed2e8df87e 100644 --- a/tools/testing/selftests/mm/Makefile +++ b/tools/testing/selftests/mm/Makefile @@ -34,6 +34,7 @@ endif MAKEFLAGS += --no-builtin-rules CFLAGS = -Wall -O2 -I $(top_srcdir) $(EXTRA_CFLAGS) $(KHDR_INCLUDES) $(TOOLS_INCLUDES) +CFLAGS += -Wunreachable-code -Wunused -Wunused-parameter -Wunused-function -Wunused-variable LDLIBS = -lrt -lpthread -lm # Some distributions (such as Ubuntu) configure GCC so that _FORTIFY_SOURCE is -- 2.39.5
Re: [PATCH 01/16] selftests/mm: remove argc and argv unused parameters
On Thu, Jan 09, 2025 at 10:38:27PM +0500, Muhammad Usama Anjum wrote:
> Remove the following warnings by removing unused argc and argv
> parameters:
> In function ‘main’:
> warning: unused parameter ‘argc’ [-Wunused-parameter]
> 158 | int main(int argc, char *argv[])
> | ^~~~
> warning: unused parameter ‘argv’ [-Wunused-parameter]
> 158 | int main(int argc, char *argv[])
> |~~^~
>
> Signed-off-by: Muhammad Usama Anjum
> ---
> tools/testing/selftests/mm/compaction_test.c | 2 +-
> tools/testing/selftests/mm/cow.c | 2 +-
> tools/testing/selftests/mm/droppable.c | 2 +-
> tools/testing/selftests/mm/gup_longterm.c | 2 +-
> tools/testing/selftests/mm/hugepage-vmemmap.c | 2 +-
> tools/testing/selftests/mm/hugetlb-madvise.c | 2 +-
> tools/testing/selftests/mm/hugetlb-soft-offline.c | 2 +-
> tools/testing/selftests/mm/madv_populate.c | 2 +-
> tools/testing/selftests/mm/map_populate.c | 2 +-
> tools/testing/selftests/mm/memfd_secret.c | 2 +-
> tools/testing/selftests/mm/mlock-random-test.c | 2 +-
> tools/testing/selftests/mm/mlock2-tests.c | 2 +-
> tools/testing/selftests/mm/on-fault-limit.c| 2 +-
> tools/testing/selftests/mm/pkey_sighandler_tests.c | 2 +-
> tools/testing/selftests/mm/soft-dirty.c| 2 +-
> tools/testing/selftests/mm/uffd-wp-mremap.c| 2 +-
> tools/testing/selftests/mm/virtual_address_range.c | 2 +-
> 17 files changed, 17 insertions(+), 17 deletions(-)
>
> diff --git a/tools/testing/selftests/mm/compaction_test.c
> b/tools/testing/selftests/mm/compaction_test.c
> index 2c3a0eb6b22d3..8d23b698ce9db 100644
> --- a/tools/testing/selftests/mm/compaction_test.c
> +++ b/tools/testing/selftests/mm/compaction_test.c
> @@ -194,7 +194,7 @@ int set_zero_hugepages(unsigned long
> *initial_nr_hugepages)
> return ret;
> }
>
> -int main(int argc, char **argv)
> +int main(void)
> {
> struct rlimit lim;
> struct map_list *list = NULL, *entry;
> diff --git a/tools/testing/selftests/mm/cow.c
> b/tools/testing/selftests/mm/cow.c
> index 1238e1c5aae15..ea00c85c76caa 100644
> --- a/tools/testing/selftests/mm/cow.c
> +++ b/tools/testing/selftests/mm/cow.c
> @@ -1769,7 +1769,7 @@ static int tests_per_non_anon_test_case(void)
> return tests;
> }
>
> -int main(int argc, char **argv)
> +int main(void)
> {
> int err;
> struct thp_settings default_settings;
> diff --git a/tools/testing/selftests/mm/droppable.c
> b/tools/testing/selftests/mm/droppable.c
> index f3d9ecf96890a..90ea6377810c5 100644
> --- a/tools/testing/selftests/mm/droppable.c
> +++ b/tools/testing/selftests/mm/droppable.c
> @@ -15,7 +15,7 @@
>
> #include "../kselftest.h"
>
> -int main(int argc, char *argv[])
> +int main(void)
> {
> size_t alloc_size = 134217728;
> size_t page_size = getpagesize();
> diff --git a/tools/testing/selftests/mm/gup_longterm.c
> b/tools/testing/selftests/mm/gup_longterm.c
> index 9423ad439a614..03a31dcb57577 100644
> --- a/tools/testing/selftests/mm/gup_longterm.c
> +++ b/tools/testing/selftests/mm/gup_longterm.c
> @@ -444,7 +444,7 @@ static int tests_per_test_case(void)
> return 3 + nr_hugetlbsizes;
> }
>
> -int main(int argc, char **argv)
> +int main(void)
> {
> int i, err;
>
> diff --git a/tools/testing/selftests/mm/hugepage-vmemmap.c
> b/tools/testing/selftests/mm/hugepage-vmemmap.c
> index df366a4d1b92d..23e97e552057d 100644
> --- a/tools/testing/selftests/mm/hugepage-vmemmap.c
> +++ b/tools/testing/selftests/mm/hugepage-vmemmap.c
> @@ -87,7 +87,7 @@ static int check_page_flags(unsigned long pfn)
> return 0;
> }
>
> -int main(int argc, char **argv)
> +int main(void)
> {
> void *addr;
> unsigned long pfn;
> diff --git a/tools/testing/selftests/mm/hugetlb-madvise.c
> b/tools/testing/selftests/mm/hugetlb-madvise.c
> index e74107185324f..43f16c12c8e9a 100644
> --- a/tools/testing/selftests/mm/hugetlb-madvise.c
> +++ b/tools/testing/selftests/mm/hugetlb-madvise.c
> @@ -58,7 +58,7 @@ void read_fault_pages(void *addr, unsigned long nr_pages)
> }
> }
>
> -int main(int argc, char **argv)
> +int main(int __attribute__((unused)) argc, char **argv)
Can we add a macro in kselftest.h for "__unused" like the kernel already
does? Then instead of removing args, we can just mark them, like you're
doing here.
--
Kees Cook
[PATCH 12/16] selftests/mm: khugepaged: Fix type mismatch warnings
Fix type mismatch warnings.
Signed-off-by: Muhammad Usama Anjum
---
tools/testing/selftests/mm/khugepaged.c | 16
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/tools/testing/selftests/mm/khugepaged.c
b/tools/testing/selftests/mm/khugepaged.c
index 8a4d34cce36b2..dde7c7fbbac2c 100644
--- a/tools/testing/selftests/mm/khugepaged.c
+++ b/tools/testing/selftests/mm/khugepaged.c
@@ -140,7 +140,7 @@ static void get_finfo(const char *dir)
exit(EXIT_FAILURE);
}
if (snprintf(finfo.path, sizeof(finfo.path), "%s/" TEST_FILE,
-finfo.dir) >= sizeof(finfo.path)) {
+finfo.dir) >= (signed int)sizeof(finfo.path)) {
printf("%s: Pathname is too long\n", __func__);
exit(EXIT_FAILURE);
}
@@ -155,7 +155,7 @@ static void get_finfo(const char *dir)
/* Find owning device's queue/read_ahead_kb control */
if (snprintf(path, sizeof(path), "/sys/dev/block/%d:%d/uevent",
major(path_stat.st_dev), minor(path_stat.st_dev))
- >= sizeof(path)) {
+ >= (signed int)sizeof(path)) {
printf("%s: Pathname is too long\n", __func__);
exit(EXIT_FAILURE);
}
@@ -169,7 +169,7 @@ static void get_finfo(const char *dir)
sizeof(finfo.dev_queue_read_ahead_path),
"/sys/dev/block/%d:%d/queue/read_ahead_kb",
major(path_stat.st_dev), minor(path_stat.st_dev))
- >= sizeof(finfo.dev_queue_read_ahead_path)) {
+ >= (signed int)sizeof(finfo.dev_queue_read_ahead_path)) {
printf("%s: Pathname is too long\n", __func__);
exit(EXIT_FAILURE);
}
@@ -197,7 +197,7 @@ static void get_finfo(const char *dir)
if (snprintf(finfo.dev_queue_read_ahead_path,
sizeof(finfo.dev_queue_read_ahead_path),
"/sys/block/%s/queue/read_ahead_kb",
-str) >=
sizeof(finfo.dev_queue_read_ahead_path)) {
+str) >= (signed
int)sizeof(finfo.dev_queue_read_ahead_path)) {
printf("%s: Pathname is too long\n", __func__);
exit(EXIT_FAILURE);
}
@@ -271,7 +271,7 @@ static void *alloc_mapping(int nr)
static void fill_memory(int *p, unsigned long start, unsigned long end)
{
- int i;
+ unsigned int i;
for (i = start / page_size; i < end / page_size; i++)
p[i * page_size / sizeof(*p)] = i + 0xdead;
@@ -333,10 +333,10 @@ static void *alloc_hpage(struct mem_ops *ops)
static void validate_memory(int *p, unsigned long start, unsigned long end)
{
- int i;
+ unsigned int i;
for (i = start / page_size; i < end / page_size; i++) {
- if (p[i * page_size / sizeof(*p)] != i + 0xdead) {
+ if ((unsigned int)p[i * page_size / sizeof(*p)] != i +
0xdead) {
printf("Page %d is corrupted: %#x\n",
i, p[i * page_size / sizeof(*p)]);
exit(EXIT_FAILURE);
@@ -537,7 +537,7 @@ static void madvise_collapse(const char *msg, char *p, int
nr_hpages,
static bool wait_for_scan(const char *msg, char *p, int nr_hpages,
struct mem_ops *ops)
{
- int full_scans;
+ unsigned int full_scans;
int timeout = 6; /* 3 seconds */
/* Sanity check */
--
2.39.5
[PATCH 15/16] selftests/mm: uffd-*: Fix all type mismatch warnings
Fix all type mismatch warnings in all uffd-* files.
Signed-off-by: Muhammad Usama Anjum
---
tools/testing/selftests/mm/uffd-common.c | 6 +++---
tools/testing/selftests/mm/uffd-stress.c | 2 +-
tools/testing/selftests/mm/uffd-unit-tests.c | 6 +++---
3 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/tools/testing/selftests/mm/uffd-common.c
b/tools/testing/selftests/mm/uffd-common.c
index c15674a60f299..47bdcb47481ae 100644
--- a/tools/testing/selftests/mm/uffd-common.c
+++ b/tools/testing/selftests/mm/uffd-common.c
@@ -450,7 +450,7 @@ void uffd_handle_page_fault(struct uffd_msg *msg, struct
uffd_args *args)
args->wp_faults++;
} else if (msg->arg.pagefault.flags & UFFD_PAGEFAULT_FLAG_MINOR) {
uint8_t *area;
- int b;
+ unsigned int b;
/*
* Minor page faults
@@ -621,7 +621,7 @@ int __copy_page(int ufd, unsigned long offset, bool retry,
bool wp)
err("UFFDIO_COPY error: %"PRId64,
(int64_t)uffdio_copy.copy);
wake_range(ufd, uffdio_copy.dst, page_size);
- } else if (uffdio_copy.copy != page_size) {
+ } else if (uffdio_copy.copy != (signed long)page_size) {
err("UFFDIO_COPY error: %"PRId64, (int64_t)uffdio_copy.copy);
} else {
if (test_uffdio_copy_eexist && retry) {
@@ -655,7 +655,7 @@ int move_page(int ufd, unsigned long offset, unsigned long
len)
err("UFFDIO_MOVE error: %"PRId64,
(int64_t)uffdio_move.move);
wake_range(ufd, uffdio_move.dst, len);
- } else if (uffdio_move.move != len) {
+ } else if (uffdio_move.move != (signed long)len) {
err("UFFDIO_MOVE error: %"PRId64, (int64_t)uffdio_move.move);
} else
return 1;
diff --git a/tools/testing/selftests/mm/uffd-stress.c
b/tools/testing/selftests/mm/uffd-stress.c
index 1270ff3104cc6..5509ec32c3290 100644
--- a/tools/testing/selftests/mm/uffd-stress.c
+++ b/tools/testing/selftests/mm/uffd-stress.c
@@ -77,7 +77,7 @@ static void usage(void)
static void uffd_stats_reset(struct uffd_args *args, unsigned long n_cpus)
{
- int i;
+ unsigned int i;
for (i = 0; i < n_cpus; i++) {
args[i].cpu = i;
diff --git a/tools/testing/selftests/mm/uffd-unit-tests.c
b/tools/testing/selftests/mm/uffd-unit-tests.c
index 81b315bedb744..c3d59ec754040 100644
--- a/tools/testing/selftests/mm/uffd-unit-tests.c
+++ b/tools/testing/selftests/mm/uffd-unit-tests.c
@@ -917,7 +917,7 @@ static bool do_uffdio_zeropage(int ufd, bool has_zeropage)
else if (res != -EINVAL)
err("UFFDIO_ZEROPAGE not -EINVAL");
} else if (has_zeropage) {
- if (res != page_size)
+ if (res != (signed long)page_size)
err("UFFDIO_ZEROPAGE unexpected size");
else
retry_uffdio_zeropage(ufd, &uffdio_zeropage);
@@ -949,7 +949,7 @@ uffd_register_detect_zeropage(int uffd, void *addr,
uint64_t len)
static void uffd_zeropage_test(uffd_test_args_t __attribute__((unused)) *args)
{
bool has_zeropage;
- int i;
+ unsigned int i;
has_zeropage = uffd_register_detect_zeropage(uffd, area_dst, page_size);
if (area_dst_alias)
@@ -997,7 +997,7 @@ static void do_uffdio_poison(int uffd, unsigned long offset)
if (ret)
err("UFFDIO_POISON error: %"PRId64, (int64_t)res);
- else if (res != page_size)
+ else if (res != (signed long)page_size)
err("UFFDIO_POISON unexpected size: %"PRId64, (int64_t)res);
}
--
2.39.5
[PATCH 14/16] selftests/mm: thuge-gen: Fix type mismatch warnings
Fix all the found type mismatch warnings.
Signed-off-by: Muhammad Usama Anjum
---
tools/testing/selftests/mm/thuge-gen.c | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/tools/testing/selftests/mm/thuge-gen.c
b/tools/testing/selftests/mm/thuge-gen.c
index e4370b79b62ff..515b89ac4eb59 100644
--- a/tools/testing/selftests/mm/thuge-gen.c
+++ b/tools/testing/selftests/mm/thuge-gen.c
@@ -65,7 +65,7 @@ void show(unsigned long ps)
{
char buf[100];
- if (ps == getpagesize())
+ if ((signed long)ps == getpagesize())
return;
ksft_print_msg("%luMB: ", ps >> 20);
@@ -106,7 +106,7 @@ unsigned long read_sysfs(int warn, char *fmt, ...)
unsigned long read_free(unsigned long ps)
{
- return read_sysfs(ps != getpagesize(),
+ return read_sysfs((signed long)ps != getpagesize(),
"/sys/kernel/mm/hugepages/hugepages-%lukB/free_hugepages",
ps >> 10);
}
@@ -126,7 +126,7 @@ void test_mmap(unsigned long size, unsigned flags)
after = read_free(size);
show(size);
- ksft_test_result(size == getpagesize() || (before - after) == NUM_PAGES,
+ ksft_test_result((signed long)size == getpagesize() || (before - after)
== NUM_PAGES,
"%s mmap\n", __func__);
if (munmap(map, size * NUM_PAGES))
@@ -164,7 +164,7 @@ void test_shmget(unsigned long size, unsigned flags)
after = read_free(size);
show(size);
- ksft_test_result(size == getpagesize() || (before - after) == NUM_PAGES,
+ ksft_test_result((signed long)size == getpagesize() || (before - after)
== NUM_PAGES,
"%s: mmap\n", __func__);
if (shmdt(map))
ksft_exit_fail_msg("%s: shmdt: %s\n", __func__,
strerror(errno));
@@ -173,7 +173,7 @@ void test_shmget(unsigned long size, unsigned flags)
void find_pagesizes(void)
{
unsigned long largest = getpagesize();
- int i;
+ unsigned int i;
glob_t g;
glob("/sys/kernel/mm/hugepages/hugepages-*kB", 0, NULL, &g);
--
2.39.5
Re: [PATCH 01/16] selftests/mm: remove argc and argv unused parameters
On 1/9/25 10:42 PM, Kees Cook wrote:
> On Thu, Jan 09, 2025 at 10:38:27PM +0500, Muhammad Usama Anjum wrote:
>> Remove the following warnings by removing unused argc and argv
>> parameters:
>> In function ‘main’:
>> warning: unused parameter ‘argc’ [-Wunused-parameter]
>> 158 | int main(int argc, char *argv[])
>> | ^~~~
>> warning: unused parameter ‘argv’ [-Wunused-parameter]
>> 158 | int main(int argc, char *argv[])
>> |~~^~
>>
>> Signed-off-by: Muhammad Usama Anjum
>> ---
>> tools/testing/selftests/mm/compaction_test.c | 2 +-
>> tools/testing/selftests/mm/cow.c | 2 +-
>> tools/testing/selftests/mm/droppable.c | 2 +-
>> tools/testing/selftests/mm/gup_longterm.c | 2 +-
>> tools/testing/selftests/mm/hugepage-vmemmap.c | 2 +-
>> tools/testing/selftests/mm/hugetlb-madvise.c | 2 +-
>> tools/testing/selftests/mm/hugetlb-soft-offline.c | 2 +-
>> tools/testing/selftests/mm/madv_populate.c | 2 +-
>> tools/testing/selftests/mm/map_populate.c | 2 +-
>> tools/testing/selftests/mm/memfd_secret.c | 2 +-
>> tools/testing/selftests/mm/mlock-random-test.c | 2 +-
>> tools/testing/selftests/mm/mlock2-tests.c | 2 +-
>> tools/testing/selftests/mm/on-fault-limit.c| 2 +-
>> tools/testing/selftests/mm/pkey_sighandler_tests.c | 2 +-
>> tools/testing/selftests/mm/soft-dirty.c| 2 +-
>> tools/testing/selftests/mm/uffd-wp-mremap.c| 2 +-
>> tools/testing/selftests/mm/virtual_address_range.c | 2 +-
>> 17 files changed, 17 insertions(+), 17 deletions(-)
>>
>> diff --git a/tools/testing/selftests/mm/compaction_test.c
>> b/tools/testing/selftests/mm/compaction_test.c
>> index 2c3a0eb6b22d3..8d23b698ce9db 100644
>> --- a/tools/testing/selftests/mm/compaction_test.c
>> +++ b/tools/testing/selftests/mm/compaction_test.c
>> @@ -194,7 +194,7 @@ int set_zero_hugepages(unsigned long
>> *initial_nr_hugepages)
>> return ret;
>> }
>>
>> -int main(int argc, char **argv)
>> +int main(void)
>> {
>> struct rlimit lim;
>> struct map_list *list = NULL, *entry;
>> diff --git a/tools/testing/selftests/mm/cow.c
>> b/tools/testing/selftests/mm/cow.c
>> index 1238e1c5aae15..ea00c85c76caa 100644
>> --- a/tools/testing/selftests/mm/cow.c
>> +++ b/tools/testing/selftests/mm/cow.c
>> @@ -1769,7 +1769,7 @@ static int tests_per_non_anon_test_case(void)
>> return tests;
>> }
>>
>> -int main(int argc, char **argv)
>> +int main(void)
>> {
>> int err;
>> struct thp_settings default_settings;
>> diff --git a/tools/testing/selftests/mm/droppable.c
>> b/tools/testing/selftests/mm/droppable.c
>> index f3d9ecf96890a..90ea6377810c5 100644
>> --- a/tools/testing/selftests/mm/droppable.c
>> +++ b/tools/testing/selftests/mm/droppable.c
>> @@ -15,7 +15,7 @@
>>
>> #include "../kselftest.h"
>>
>> -int main(int argc, char *argv[])
>> +int main(void)
>> {
>> size_t alloc_size = 134217728;
>> size_t page_size = getpagesize();
>> diff --git a/tools/testing/selftests/mm/gup_longterm.c
>> b/tools/testing/selftests/mm/gup_longterm.c
>> index 9423ad439a614..03a31dcb57577 100644
>> --- a/tools/testing/selftests/mm/gup_longterm.c
>> +++ b/tools/testing/selftests/mm/gup_longterm.c
>> @@ -444,7 +444,7 @@ static int tests_per_test_case(void)
>> return 3 + nr_hugetlbsizes;
>> }
>>
>> -int main(int argc, char **argv)
>> +int main(void)
>> {
>> int i, err;
>>
>> diff --git a/tools/testing/selftests/mm/hugepage-vmemmap.c
>> b/tools/testing/selftests/mm/hugepage-vmemmap.c
>> index df366a4d1b92d..23e97e552057d 100644
>> --- a/tools/testing/selftests/mm/hugepage-vmemmap.c
>> +++ b/tools/testing/selftests/mm/hugepage-vmemmap.c
>> @@ -87,7 +87,7 @@ static int check_page_flags(unsigned long pfn)
>> return 0;
>> }
>>
>> -int main(int argc, char **argv)
>> +int main(void)
>> {
>> void *addr;
>> unsigned long pfn;
>> diff --git a/tools/testing/selftests/mm/hugetlb-madvise.c
>> b/tools/testing/selftests/mm/hugetlb-madvise.c
>> index e74107185324f..43f16c12c8e9a 100644
>> --- a/tools/testing/selftests/mm/hugetlb-madvise.c
>> +++ b/tools/testing/selftests/mm/hugetlb-madvise.c
>> @@ -58,7 +58,7 @@ void read_fault_pages(void *addr, unsigned long nr_pages)
>> }
>> }
>>
>> -int main(int argc, char **argv)
>> +int main(int __attribute__((unused)) argc, char **argv)
>
> Can we add a macro in kselftest.h for "__unused" like the kernel already
> does?
It can be done. But as there kselftest patches aren't regulated as much as
other kernel patches, people may still not use this macro and use the gcc
attribute directly. Let's see what others have to say.
> Then instead of removing args, we can just mark them, like you're
> doing here.
In this case, argv is being used while argc isn't being used. I didn't
find a way to keep argv and remove argc. Hence I marked argc as unused.
For the all other case, why shou
[PATCH 09/16] selftests/mm: hugetlb-madvise: fix type mismatch issues
Fix type mismatch warnings:
hugetlb-madvise.c:30:25: warning: comparison of integer expressions of
different signedness: ‘int’ and ‘long unsigned int’ [-Wsign-compare]
30 | if (fhp != (exp_free)) {
\
| ^~
hugetlb-madvise.c:114:9: note: in expansion of macro ‘validate_free_pages’
114 | validate_free_pages(free_hugepages - NR_HUGE_PAGES);
| ^~~
Signed-off-by: Muhammad Usama Anjum
---
tools/testing/selftests/mm/hugetlb-madvise.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/testing/selftests/mm/hugetlb-madvise.c
b/tools/testing/selftests/mm/hugetlb-madvise.c
index 43f16c12c8e9a..8f527084858d0 100644
--- a/tools/testing/selftests/mm/hugetlb-madvise.c
+++ b/tools/testing/selftests/mm/hugetlb-madvise.c
@@ -26,7 +26,7 @@
#define validate_free_pages(exp_free) \
do {\
- int fhp = get_free_hugepages(); \
+ unsigned int fhp = get_free_hugepages();\
if (fhp != (exp_free)) {\
printf("Unexpected number of free huge "\
"pages line %d\n", __LINE__); \
--
2.39.5
Re: [PATCH net 2/2] vsock/bpf: return early if transport is not assigned
On 1/8/25 19:06, Stefano Garzarella wrote:
> Some of the core functions can only be called if the transport
> has been assigned.
>
> As Michal reported, a socket might have the transport at NULL,
> for example after a failed connect(), causing the following trace:
>
> BUG: kernel NULL pointer dereference, address: 00a0
> #PF: supervisor read access in kernel mode
> #PF: error_code(0x) - not-present page
> PGD 12faf8067 P4D 12faf8067 PUD 113670067 PMD 0
> Oops: Oops: [#1] PREEMPT SMP NOPTI
> CPU: 15 UID: 0 PID: 1198 Comm: a.out Not tainted 6.13.0-rc2+
> RIP: 0010:vsock_connectible_has_data+0x1f/0x40
> Call Trace:
> vsock_bpf_recvmsg+0xca/0x5e0
> sock_recvmsg+0xb9/0xc0
> __sys_recvfrom+0xb3/0x130
> __x64_sys_recvfrom+0x20/0x30
> do_syscall_64+0x93/0x180
> entry_SYSCALL_64_after_hwframe+0x76/0x7e
>
> So we need to check the `vsk->transport` in vsock_bpf_recvmsg(),
> especially for connected sockets (stream/seqpacket) as we already
> do in __vsock_connectible_recvmsg().
>
> Fixes: 634f1a7110b4 ("vsock: support sockmap")
> Reported-by: Michal Luczaj
> Closes:
> https://lore.kernel.org/netdev/5ca20d4c-1017-49c2-9516-f6f75fd33...@rbox.co/
> Signed-off-by: Stefano Garzarella
Tested-by: Michal Luczaj
> ---
> net/vmw_vsock/vsock_bpf.c | 9 +
> 1 file changed, 9 insertions(+)
>
> diff --git a/net/vmw_vsock/vsock_bpf.c b/net/vmw_vsock/vsock_bpf.c
> index 4aa6e74ec295..f201d9eca1df 100644
> --- a/net/vmw_vsock/vsock_bpf.c
> +++ b/net/vmw_vsock/vsock_bpf.c
> @@ -77,6 +77,7 @@ static int vsock_bpf_recvmsg(struct sock *sk, struct msghdr
> *msg,
>size_t len, int flags, int *addr_len)
> {
> struct sk_psock *psock;
> + struct vsock_sock *vsk;
> int copied;
>
> psock = sk_psock_get(sk);
> @@ -84,6 +85,13 @@ static int vsock_bpf_recvmsg(struct sock *sk, struct
> msghdr *msg,
> return __vsock_recvmsg(sk, msg, len, flags);
>
> lock_sock(sk);
> + vsk = vsock_sk(sk);
> +
> + if (!vsk->transport) {
> + copied = -ENODEV;
> + goto out;
> + }
> +
> if (vsock_has_data(sk, psock) && sk_psock_queue_empty(psock)) {
> release_sock(sk);
> sk_psock_put(sk, psock);
> @@ -108,6 +116,7 @@ static int vsock_bpf_recvmsg(struct sock *sk, struct
> msghdr *msg,
> copied = sk_msg_recvmsg(sk, psock, msg, len, flags);
> }
>
> +out:
> release_sock(sk);
> sk_psock_put(sk, psock);
>
Re: [PATCH net 1/2] vsock/virtio: discard packets if the transport changes
On Thu, Jan 09, 2025 at 02:34:28PM +0100, Michal Luczaj wrote:
On 1/8/25 19:06, Stefano Garzarella wrote:
If the socket has been de-assigned or assigned to another transport,
we must discard any packets received because they are not expected
and would cause issues when we access vsk->transport.
A possible scenario is described by Hyunwoo Kim in the attached link,
where after a first connect() interrupted by a signal, and a second
connect() failed, we can find `vsk->transport` at NULL, leading to a
NULL pointer dereference.
Fixes: c0cfa2d8a788 ("vsock: add multi-transports support")
Reported-by: Hyunwoo Kim
Reported-by: Wongi Lee
Closes:
https://lore.kernel.org/netdev/Z2LvdTTQR7dBmPb5@v4bel-B760M-AORUS-ELITE-AX/
Signed-off-by: Stefano Garzarella
---
net/vmw_vsock/virtio_transport_common.c | 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/net/vmw_vsock/virtio_transport_common.c
b/net/vmw_vsock/virtio_transport_common.c
index 9acc13ab3f82..51a494b69be8 100644
--- a/net/vmw_vsock/virtio_transport_common.c
+++ b/net/vmw_vsock/virtio_transport_common.c
@@ -1628,8 +1628,11 @@ void virtio_transport_recv_pkt(struct virtio_transport
*t,
lock_sock(sk);
- /* Check if sk has been closed before lock_sock */
- if (sock_flag(sk, SOCK_DONE)) {
+ /* Check if sk has been closed or assigned to another transport before
+* lock_sock (note: listener sockets are not assigned to any transport)
+*/
+ if (sock_flag(sk, SOCK_DONE) ||
+ (sk->sk_state != TCP_LISTEN && vsk->transport != &t->transport)) {
(void)virtio_transport_reset_no_sock(t, skb);
release_sock(sk);
sock_put(sk);
FWIW, I've tried simplifying Hyunwoo's repro to toy with some tests. Ended
up with
```
from threading import *
from socket import *
from signal import *
def listener(tid):
while True:
s = socket(AF_VSOCK, SOCK_SEQPACKET)
s.bind((1, 1234))
s.listen()
pthread_kill(tid, SIGUSR1)
signal(SIGUSR1, lambda *args: None)
Thread(target=listener, args=[get_ident()]).start()
while True:
c = socket(AF_VSOCK, SOCK_SEQPACKET)
c.connect_ex((1, 1234))
c.connect_ex((42, 1234))
```
which gives me splats with or without this patch.
That said, when I apply this patch, but drop the `sk->sk_state !=
TCP_LISTEN &&`: no more splats.
We can't drop `sk->sk_state != TCP_LISTEN &&` because listener socket
doesn't have any transport (vsk->transport == NULL), so every connection
request will receive an error, so maybe this is the reason of no splats.
I'm cooking some more patches to fix Hyunwoo's scenario handling better
the close work when the virtio destructor is called.
I'll run your reproduces to test it, thanks for that!
Stefano
Re: [PATCH] arm64: dts: qcom: msm8916-samsung-serranove: Add display panel
On Thu, 14 Nov 2024 23:07:18 +0100, Jakob Hauser wrote: > Add the Samsung S6E88A0-AMS427AP24 panel to the device tree for the > Samsung Galaxy S4 Mini Value Edition. By default the panel displays > everything horizontally flipped, so add "flip-horizontal" to the panel > node to correct that. > > Applied, thanks! [1/1] arm64: dts: qcom: msm8916-samsung-serranove: Add display panel commit: 46316370e9257647d81c13782a6201a2256d6f1d Best regards, -- Bjorn Andersson
Re: [PATCH net 0/2] bond: fix xfrm offload feature during init
On Thu, Jan 09, 2025 at 09:26:38AM +0800, Jianbo Liu wrote:
>
>
> On 1/8/2025 3:14 PM, Hangbin Liu wrote:
> > On Wed, Jan 08, 2025 at 11:40:05AM +0800, Jianbo Liu wrote:
> > >
> > >
> > > On 1/8/2025 10:46 AM, Hangbin Liu wrote:
> > > > On Mon, Jan 06, 2025 at 10:47:16AM +, Hangbin Liu wrote:
> > > > > On Thu, Jan 02, 2025 at 11:33:34AM +0800, Jianbo Liu wrote:
> > > > > > > > Re-locking doesn't look great, glancing at the code I don't see
> > > > > > > > any
> > > > > > > > obvious better workarounds. Easiest fix would be to don't let
> > > > > > > > the
> > > > > > > > drivers sleep in the callbacks and then we can go back to a
> > > > > > > > spin lock.
> > > > > > > > Maybe nvidia people have better ideas, I'm not familiar with
> > > > > > > > this
> > > > > > > > offload.
> > > > > > >
> > > > > > > I don't know how to disable bonding sleeping since we use
> > > > > > > mutex_lock now.
> > > > > > > Hi Jianbo, do you have any idea?
> > > > > > >
> > > > > >
> > > > > > I think we should allow drivers to sleep in the callbacks. So,
> > > > > > maybe it's
> > > > > > better to move driver's xdo_dev_state_delete out of state's spin
> > > > > > lock.
> > > > >
> > > > > I just check the code, xfrm_dev_state_delete() and later
> > > > > dev->xfrmdev_ops->xdo_dev_state_delete(x) have too many xfrm_state x
> > > > > checks. Can we really move it out of spin lock from
> > > > > xfrm_state_delete()
> > > >
> > > > I tried to move the mutex lock code to a work queue, but found we need
> > > > to
> > > > check (ipsec->xs == xs) in bonding. So we still need xfrm_state x
> > > > during bond
> > >
> > > Maybe I miss something, but why need to hold spin lock. You can keep xfrm
> > > state by its refcnt.
> >
> > Do you mean move the xfrm_dev_state_delete() out of spin lock directly like:
> >
>
> Yes. Not feasible?
>
> > diff --git a/net/xfrm/xfrm_state.c b/net/xfrm/xfrm_state.c
> > index 67ca7ac955a3..6881ddeb4360 100644
> > --- a/net/xfrm/xfrm_state.c
> > +++ b/net/xfrm/xfrm_state.c
> > @@ -766,13 +766,6 @@ int __xfrm_state_delete(struct xfrm_state *x)
> > if (x->encap_sk)
> > sock_put(rcu_dereference_raw(x->encap_sk));
> > - xfrm_dev_state_delete(x);
> > -
> > - /* All xfrm_state objects are created by xfrm_state_alloc.
> > -* The xfrm_state_alloc call gives a reference, and that
> > -* is what we are dropping here.
> > -*/
> > - xfrm_state_put(x);
> > err = 0;
> > }
> > @@ -787,8 +780,20 @@ int xfrm_state_delete(struct xfrm_state *x)
> > spin_lock_bh(&x->lock);
> > err = __xfrm_state_delete(x);
> > spin_unlock_bh(&x->lock);
> > + if (err)
> > + return err;
> > - return err;
> > + if (x->km.state == XFRM_STATE_DEAD) {
> > + xfrm_dev_state_delete(x);
> > +
> > + /* All xfrm_state objects are created by xfrm_state_alloc.
> > +* The xfrm_state_alloc call gives a reference, and that
> > +* is what we are dropping here.
> > +*/
> > + xfrm_state_put(x);
> > + }
> > +
> > + return 0;
> > }
> > EXPORT_SYMBOL(xfrm_state_delete);
> >
> > Then why we need the spin lock in xfrm_state_delete?
> >
>
> No, we don't need. But I am trying to understand what you said in your last
> email about adding a new lock, or unlocking spin lock in
I *thought* we need the spin lock in xfrm_state_delete(). So to protect
xfrm_state,
we need a new lock. Although it looks redundant. e.g.
int xfrm_state_delete(struct xfrm_state *x)
{
int err;
spin_lock_bh(&x->lock);
err = __xfrm_state_delete(x);
spin_unlock_bh(&x->lock);
if (err)
return err;
another_lock(&x->other_lock)
if (x->km.state == XFRM_STATE_DEAD) {
xfrm_dev_state_delete(x);
xfrm_state_put(x);
}
another_unlock(&x->other_lock)
return 0;
}
> bond_ipsec_del_sa(). Anything I missed?
The unlock spin lock in bond_ipsec_del_sa looks like
https://lore.kernel.org/netdev/Z1vfsAyuxcohT7th@fedora/
Thanks
Hangbin
Re: [PATCH v6 12/13] media: i2c: imx214: Fix clock handling on probe error or remove
Hi André,
On Wed, Jan 08, 2025 at 10:46:51PM +0100, André Apitzsch wrote:
> Hi Sakari,
>
> Am Mittwoch, dem 08.01.2025 um 11:51 + schrieb Sakari Ailus:
> > Hi André,
> >
> > Thanks for the update.
> >
> > On Fri, Dec 20, 2024 at 02:26:11PM +0100, André Apitzsch via B4 Relay
> > wrote:
> > > From: André Apitzsch
> > >
> > > The driver should disable or unprepare the clock on probe error and
> > > on
> > > removing the module.
> > >
> > > Remove regulator_bulk_disable(), because it is called by
> > > imx214_power_off().
> > >
> > > Acked-by: Ricardo Ribalda
> > > Signed-off-by: André Apitzsch
> >
> > This and the next one appear to be bugfixes. I think it'd be
> > reasonable to backport them. Any idea where this got broken, for the
> > Fixes: tag?
> > The bug fixed by the last patch was probably introduced with the
> > driver?
> >
>
> I think both, the missing imx214_power_off() and the wrong link
> frequency where introduced with
> > 436190596241 ("media: imx214: Add imx214 camera sensor driver")
>
> The first one could also be considered as a fix for the incomplete
> > 9bc92332cc3f ("media: imx214: Fix the error handling in imx214_probe()")
This one seems to have missed review. :-(
>
> Shall I submit a new version with the Fixes: tag?
I checked the fix again and it seems the problem isn't as simple:
pm_runtime_idle() call leads to powering the device off. It needs to either
be resumed first or the call to imx214_power_off() be omitted.
I tried to find a good example but the few I found are buggy. :-( Getting
runtime PM right is just hard. I'll drop the last patch and post another to
address the problem (plus the intended example drivers) if you're fine with
that.
--
Kind regards,
Sakari Ailus
Re: [PATCH net 0/2] bond: fix xfrm offload feature during init
On 1/9/2025 4:37 PM, Hangbin Liu wrote:
On Thu, Jan 09, 2025 at 09:26:38AM +0800, Jianbo Liu wrote:
On 1/8/2025 3:14 PM, Hangbin Liu wrote:
On Wed, Jan 08, 2025 at 11:40:05AM +0800, Jianbo Liu wrote:
On 1/8/2025 10:46 AM, Hangbin Liu wrote:
On Mon, Jan 06, 2025 at 10:47:16AM +, Hangbin Liu wrote:
On Thu, Jan 02, 2025 at 11:33:34AM +0800, Jianbo Liu wrote:
Re-locking doesn't look great, glancing at the code I don't see any
obvious better workarounds. Easiest fix would be to don't let the
drivers sleep in the callbacks and then we can go back to a spin lock.
Maybe nvidia people have better ideas, I'm not familiar with this
offload.
I don't know how to disable bonding sleeping since we use mutex_lock now.
Hi Jianbo, do you have any idea?
I think we should allow drivers to sleep in the callbacks. So, maybe it's
better to move driver's xdo_dev_state_delete out of state's spin lock.
I just check the code, xfrm_dev_state_delete() and later
dev->xfrmdev_ops->xdo_dev_state_delete(x) have too many xfrm_state x
checks. Can we really move it out of spin lock from xfrm_state_delete()
I tried to move the mutex lock code to a work queue, but found we need to
check (ipsec->xs == xs) in bonding. So we still need xfrm_state x during bond
Maybe I miss something, but why need to hold spin lock. You can keep xfrm
state by its refcnt.
Do you mean move the xfrm_dev_state_delete() out of spin lock directly like:
Yes. Not feasible?
diff --git a/net/xfrm/xfrm_state.c b/net/xfrm/xfrm_state.c
index 67ca7ac955a3..6881ddeb4360 100644
--- a/net/xfrm/xfrm_state.c
+++ b/net/xfrm/xfrm_state.c
@@ -766,13 +766,6 @@ int __xfrm_state_delete(struct xfrm_state *x)
if (x->encap_sk)
sock_put(rcu_dereference_raw(x->encap_sk));
- xfrm_dev_state_delete(x);
-
- /* All xfrm_state objects are created by xfrm_state_alloc.
-* The xfrm_state_alloc call gives a reference, and that
-* is what we are dropping here.
-*/
- xfrm_state_put(x);
err = 0;
}
@@ -787,8 +780,20 @@ int xfrm_state_delete(struct xfrm_state *x)
spin_lock_bh(&x->lock);
err = __xfrm_state_delete(x);
spin_unlock_bh(&x->lock);
+ if (err)
+ return err;
- return err;
+ if (x->km.state == XFRM_STATE_DEAD) {
+ xfrm_dev_state_delete(x);
+
+ /* All xfrm_state objects are created by xfrm_state_alloc.
+* The xfrm_state_alloc call gives a reference, and that
+* is what we are dropping here.
+*/
+ xfrm_state_put(x);
+ }
+
+ return 0;
}
EXPORT_SYMBOL(xfrm_state_delete);
Then why we need the spin lock in xfrm_state_delete?
No, we don't need. But I am trying to understand what you said in your last
email about adding a new lock, or unlocking spin lock in
I *thought* we need the spin lock in xfrm_state_delete(). So to protect
xfrm_state,
But not need in bond_ipsec_del_sa() because the state still hold by
xfrm_state_hold(), right?
we need a new lock. Although it looks redundant. e.g.
int xfrm_state_delete(struct xfrm_state *x)
{
int err;
spin_lock_bh(&x->lock);
err = __xfrm_state_delete(x);
spin_unlock_bh(&x->lock);
if (err)
return err;
another_lock(&x->other_lock)
if (x->km.state == XFRM_STATE_DEAD) {
xfrm_dev_state_delete(x);
xfrm_state_put(x);
}
another_unlock(&x->other_lock)
return 0;
}
bond_ipsec_del_sa(). Anything I missed?
The unlock spin lock in bond_ipsec_del_sa looks like
https://lore.kernel.org/netdev/Z1vfsAyuxcohT7th@fedora/
Thanks
Hangbin
Re: [PATCH v4 4/4] rust: add parameter support to the `module!` macro
"Greg KH" writes: > On Thu, Jan 09, 2025 at 11:54:59AM +0100, Andreas Hindborg wrote: >> This patch includes changes required for Rust kernel modules to utilize >> module parameters. This code implements read only support for integer >> types without `sysfs` support. > > I know you want to keep this simple for now, but will you have to go and > touch all users of this when you do add the sysfs support later? sysfs > wants the mode of the file to be set here, so how do you think of that > happening? We would add the required fields to the `module!` macro as optional fields. No need to touch everyone. Leaving out the sysfs file permission field would cause the parameter to not show up in sysfs. > And don't you need that for your null block driver? Yes I need it eventually. > Also, what about all the other "types" of module parameters that are > currently able to be done, like call-back, hardware control, and unsafe? > Are we just not going to do that for rust code (no objection from me, > just wanting to be sure.) Someone told me "no dead code", so I would defer those features to when we have a user. We have blueprints for strings and arrays based on Adams earlier work. I don't imagine any rust code relying on hw param and the use in the kernel seems to be very limited. Not sure about cb params. While it might be nice for the user to be able to pass a callback to decode/sanitize , the same effect could be achieved by putting the logic elsewhere. I don't have an idea of how to implement this in Rust at the moment, but I am sure we can come up with a solution if we need to. Best regards, Andreas Hindborg
Re: [PATCH 1/3] selftests/mm: virtual_address_range: Fix error when CommitLimit < 1GiB
>
That is clear. The issue would be to figure which chunks are valid to
unmap. If something critical like the executable file is unmapped,
the process crashes. But see below.
Ah, now I see what you mean. Yes, also the stack etc. will be
problematic. So IIUC, you want to limit the munmap optimization only to
the manually mmap()ed parts.
Is it fine to rely on CONFIG_ANON_VMA_NAME?
That would make it much easier to implement.
Can you elaborate how you would do it?
First set the VMA name after mmap():
for (i = 0; i < NR_CHUNKS_LOW; i++) {
ptr[i] = mmap(NULL, MAP_CHUNK_SIZE, PROT_READ | PROT_WRITE,
MAP_NORESERVE | MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
if (ptr[i] == MAP_FAILED) {
if (validate_lower_address_hint())
ksft_exit_fail_msg("mmap unexpectedly succeeded with
hint\n");
break;
}
validate_addr(ptr[i], 0);
if (prctl(PR_SET_VMA, PR_SET_VMA_ANON_NAME, ptr[i], MAP_CHUNK_SIZE,
"virtual_address_range"))
ksft_exit_fail_msg("prctl(PR_SET_VMA_ANON_NAME) failed: %s\n",
strerror(errno));
Likely this would prevent merging of VMAs.
With a 1 GiB chunk size, and NR_CHUNKS_LOW == 128TiB, you'd already
require 128k VMAs. The default limit is frequently 64k.
We could just scan the ptr / hptr array to see if this is a manual mmap
area or not. If this takes too long, one could sort the arrays by
address and perform a binary search.
Not the most efficient way of doing it, but maybe good enough for this test?
Alternatively, store the pointer in a xarray-like tree instead of two
arrays. Requires a bit more memory ... and we'd have to find a simple
implementation we could just reuse in this test. So maybe there is a
simpler way to get it done.
--
Cheers,
David / dhildenb
Re: [PATCH net 1/2] vsock/virtio: discard packets if the transport changes
On Wed, Jan 08, 2025 at 02:31:19PM -0500, Hyunwoo Kim wrote:
On Wed, Jan 08, 2025 at 07:06:16PM +0100, Stefano Garzarella wrote:
If the socket has been de-assigned or assigned to another transport,
we must discard any packets received because they are not expected
and would cause issues when we access vsk->transport.
A possible scenario is described by Hyunwoo Kim in the attached link,
where after a first connect() interrupted by a signal, and a second
connect() failed, we can find `vsk->transport` at NULL, leading to a
NULL pointer dereference.
Fixes: c0cfa2d8a788 ("vsock: add multi-transports support")
Reported-by: Hyunwoo Kim
Reported-by: Wongi Lee
Closes:
https://lore.kernel.org/netdev/Z2LvdTTQR7dBmPb5@v4bel-B760M-AORUS-ELITE-AX/
Signed-off-by: Stefano Garzarella
---
net/vmw_vsock/virtio_transport_common.c | 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/net/vmw_vsock/virtio_transport_common.c
b/net/vmw_vsock/virtio_transport_common.c
index 9acc13ab3f82..51a494b69be8 100644
--- a/net/vmw_vsock/virtio_transport_common.c
+++ b/net/vmw_vsock/virtio_transport_common.c
@@ -1628,8 +1628,11 @@ void virtio_transport_recv_pkt(struct virtio_transport
*t,
lock_sock(sk);
- /* Check if sk has been closed before lock_sock */
- if (sock_flag(sk, SOCK_DONE)) {
+ /* Check if sk has been closed or assigned to another transport before
+* lock_sock (note: listener sockets are not assigned to any transport)
+*/
+ if (sock_flag(sk, SOCK_DONE) ||
+ (sk->sk_state != TCP_LISTEN && vsk->transport != &t->transport)) {
If a race scenario with vsock_listen() is added to the existing
race scenario, the patch can be bypassed.
In addition to the existing scenario:
```
cpu0
cpu1
socket(A)
bind(A, {cid:
VMADDR_CID_LOCAL, port: 1024})
vsock_bind()
listen(A)
vsock_listen()
socket(B)
connect(B, {cid: VMADDR_CID_LOCAL, port: 1024})
vsock_connect()
lock_sock(sk);
virtio_transport_connect()
virtio_transport_connect()
virtio_transport_send_pkt_info()
vsock_loopback_send_pkt(VIRTIO_VSOCK_OP_REQUEST)
queue_work(vsock_loopback_work)
sk->sk_state = TCP_SYN_SENT;
release_sock(sk);
vsock_loopback_work()
virtio_transport_recv_pkt(VIRTIO_VSOCK_OP_REQUEST)
sk =
vsock_find_bound_socket(&dst);
virtio_transport_recv_listen(sk, skb)
child =
vsock_create_connected(sk);
vsock_assign_transport()
vvs =
kzalloc(sizeof(*vvs), GFP_KERNEL);
vsock_insert_connected(vchild);
list_add(&vsk->connected_table, list);
virtio_transport_send_response(vchild, skb);
virtio_transport_send_pkt_info()
vsock_loopback_send_pkt(VIRTIO_VSOCK_OP_RESPONSE)
queue_work(vsock_loopback_work)
vsock_loopback_work()
virtio_transport_recv_pkt(VIRTIO_VSOCK_OP_RESPONSE)
sk =
vsock_find_bound_socket(&dst);
lock_sock(sk);
case
TCP_SYN_SENT:
virtio_transport_recv_connecting()
sk->sk_state = TCP_ESTABLISHED;
release_sock(sk);
kill(connect(B));
lock_sock(sk);
if (signal_pending(current)) {
sk->sk_state = sk->sk_state == TCP_ESTABLISHED ? TCP_CLOSING : TCP_CLOSE;
soc
Re: [PATCH v6 12/13] media: i2c: imx214: Fix clock handling on probe error or remove
On Thu, Jan 09, 2025 at 08:59:43AM +, Sakari Ailus wrote: > I tried to find a good example but the few I found are buggy. :-( Getting > runtime PM right is just hard. I'll drop the last patch and post another to I meant to say this patch, not the last one. -- Sakari Ailus
Re: [PATCH net 1/2] vsock/virtio: discard packets if the transport changes
On Thu, Jan 09, 2025 at 10:01:31AM +0100, Stefano Garzarella wrote:
> On Wed, Jan 08, 2025 at 02:31:19PM -0500, Hyunwoo Kim wrote:
> > On Wed, Jan 08, 2025 at 07:06:16PM +0100, Stefano Garzarella wrote:
> > > If the socket has been de-assigned or assigned to another transport,
> > > we must discard any packets received because they are not expected
> > > and would cause issues when we access vsk->transport.
> > >
> > > A possible scenario is described by Hyunwoo Kim in the attached link,
> > > where after a first connect() interrupted by a signal, and a second
> > > connect() failed, we can find `vsk->transport` at NULL, leading to a
> > > NULL pointer dereference.
> > >
> > > Fixes: c0cfa2d8a788 ("vsock: add multi-transports support")
> > > Reported-by: Hyunwoo Kim
> > > Reported-by: Wongi Lee
> > > Closes:
> > > https://lore.kernel.org/netdev/Z2LvdTTQR7dBmPb5@v4bel-B760M-AORUS-ELITE-AX/
> > > Signed-off-by: Stefano Garzarella
> > > ---
> > > net/vmw_vsock/virtio_transport_common.c | 7 +--
> > > 1 file changed, 5 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/net/vmw_vsock/virtio_transport_common.c
> > > b/net/vmw_vsock/virtio_transport_common.c
> > > index 9acc13ab3f82..51a494b69be8 100644
> > > --- a/net/vmw_vsock/virtio_transport_common.c
> > > +++ b/net/vmw_vsock/virtio_transport_common.c
> > > @@ -1628,8 +1628,11 @@ void virtio_transport_recv_pkt(struct
> > > virtio_transport *t,
> > >
> > > lock_sock(sk);
> > >
> > > - /* Check if sk has been closed before lock_sock */
> > > - if (sock_flag(sk, SOCK_DONE)) {
> > > + /* Check if sk has been closed or assigned to another transport before
> > > + * lock_sock (note: listener sockets are not assigned to any transport)
> > > + */
> > > + if (sock_flag(sk, SOCK_DONE) ||
> > > + (sk->sk_state != TCP_LISTEN && vsk->transport != &t->transport)) {
> >
> > If a race scenario with vsock_listen() is added to the existing
> > race scenario, the patch can be bypassed.
> >
> > In addition to the existing scenario:
> > ```
> > cpu0
> > cpu1
> >
> > socket(A)
> >
> > bind(A, {cid:
> > VMADDR_CID_LOCAL, port: 1024})
> > vsock_bind()
> >
> > listen(A)
> >
> > vsock_listen()
> > socket(B)
> >
> > connect(B, {cid: VMADDR_CID_LOCAL, port: 1024})
> >vsock_connect()
> > lock_sock(sk);
> > virtio_transport_connect()
> >virtio_transport_connect()
> > virtio_transport_send_pkt_info()
> >vsock_loopback_send_pkt(VIRTIO_VSOCK_OP_REQUEST)
> > queue_work(vsock_loopback_work)
> > sk->sk_state = TCP_SYN_SENT;
> > release_sock(sk);
> >
> > vsock_loopback_work()
> >
> > virtio_transport_recv_pkt(VIRTIO_VSOCK_OP_REQUEST)
> > sk =
> > vsock_find_bound_socket(&dst);
> >
> > virtio_transport_recv_listen(sk, skb)
> > child =
> > vsock_create_connected(sk);
> >
> > vsock_assign_transport()
> > vvs =
> > kzalloc(sizeof(*vvs), GFP_KERNEL);
> >
> > vsock_insert_connected(vchild);
> >
> > list_add(&vsk->connected_table, list);
> >
> > virtio_transport_send_response(vchild, skb);
> >
> > virtio_transport_send_pkt_info()
> >
> > vsock_loopback_send_pkt(VIRTIO_VSOCK_OP_RESPONSE)
> >
> > queue_work(vsock_loopback_work)
> >
> >
> > vsock_loopback_work()
> >
> > virtio_transport_recv_pkt(VIRTIO_VSOCK_OP_RESPONSE)
> > sk =
> > vsock_find_bound_socket(&dst);
> >
> > lock_sock(sk);
> > case
> > TCP_SYN_SENT:
> >
Re: [PATCH] virtio: console: Replace deprecated kmap_atomic with kmap_local_page
On Wed, 2025-01-08 at 19:59 -0800, David Reaver wrote: > kmap_atomic() is deprecated and should be replaced with > kmap_local_page() > [1][2]. kmap_local_page() is faster in kernels with HIGHMEM enabled, > can > take page faults, and allows preemption. > > According to [2], this replacement is safe as long as the code > between > kmap_atomic() and kunmap_atomic() does not implicitly depend on > disabling > page faults or preemption. In this patch, the only thing happening > between > mapping and unmapping the page is a memcpy, and I don't suspect it > depends > on disabling page faults or preemption. > > [1] https://lwn.net/Articles/836144/ > [2] > https://docs.kernel.org/mm/highmem.html#temporary-virtual-mappings > > Signed-off-by: David Reaver Reviewed-by: Amit Shah
Re: [PATCH net 1/2] vsock/virtio: discard packets if the transport changes
On Thu, Jan 09, 2025 at 10:01:31AM +0100, Stefano Garzarella wrote:
> On Wed, Jan 08, 2025 at 02:31:19PM -0500, Hyunwoo Kim wrote:
> > On Wed, Jan 08, 2025 at 07:06:16PM +0100, Stefano Garzarella wrote:
> > > If the socket has been de-assigned or assigned to another transport,
> > > we must discard any packets received because they are not expected
> > > and would cause issues when we access vsk->transport.
> > >
> > > A possible scenario is described by Hyunwoo Kim in the attached link,
> > > where after a first connect() interrupted by a signal, and a second
> > > connect() failed, we can find `vsk->transport` at NULL, leading to a
> > > NULL pointer dereference.
> > >
> > > Fixes: c0cfa2d8a788 ("vsock: add multi-transports support")
> > > Reported-by: Hyunwoo Kim
> > > Reported-by: Wongi Lee
> > > Closes:
> > > https://lore.kernel.org/netdev/Z2LvdTTQR7dBmPb5@v4bel-B760M-AORUS-ELITE-AX/
> > > Signed-off-by: Stefano Garzarella
> > > ---
> > > net/vmw_vsock/virtio_transport_common.c | 7 +--
> > > 1 file changed, 5 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/net/vmw_vsock/virtio_transport_common.c
> > > b/net/vmw_vsock/virtio_transport_common.c
> > > index 9acc13ab3f82..51a494b69be8 100644
> > > --- a/net/vmw_vsock/virtio_transport_common.c
> > > +++ b/net/vmw_vsock/virtio_transport_common.c
> > > @@ -1628,8 +1628,11 @@ void virtio_transport_recv_pkt(struct
> > > virtio_transport *t,
> > >
> > > lock_sock(sk);
> > >
> > > - /* Check if sk has been closed before lock_sock */
> > > - if (sock_flag(sk, SOCK_DONE)) {
> > > + /* Check if sk has been closed or assigned to another transport before
> > > + * lock_sock (note: listener sockets are not assigned to any transport)
> > > + */
> > > + if (sock_flag(sk, SOCK_DONE) ||
> > > + (sk->sk_state != TCP_LISTEN && vsk->transport != &t->transport)) {
> >
> > If a race scenario with vsock_listen() is added to the existing
> > race scenario, the patch can be bypassed.
> >
> > In addition to the existing scenario:
> > ```
> > cpu0
> > cpu1
> >
> > socket(A)
> >
> > bind(A, {cid:
> > VMADDR_CID_LOCAL, port: 1024})
> > vsock_bind()
> >
> > listen(A)
> >
> > vsock_listen()
> > socket(B)
> >
> > connect(B, {cid: VMADDR_CID_LOCAL, port: 1024})
> >vsock_connect()
> > lock_sock(sk);
> > virtio_transport_connect()
> >virtio_transport_connect()
> > virtio_transport_send_pkt_info()
> >vsock_loopback_send_pkt(VIRTIO_VSOCK_OP_REQUEST)
> > queue_work(vsock_loopback_work)
> > sk->sk_state = TCP_SYN_SENT;
> > release_sock(sk);
> >
> > vsock_loopback_work()
> >
> > virtio_transport_recv_pkt(VIRTIO_VSOCK_OP_REQUEST)
> > sk =
> > vsock_find_bound_socket(&dst);
> >
> > virtio_transport_recv_listen(sk, skb)
> > child =
> > vsock_create_connected(sk);
> >
> > vsock_assign_transport()
> > vvs =
> > kzalloc(sizeof(*vvs), GFP_KERNEL);
> >
> > vsock_insert_connected(vchild);
> >
> > list_add(&vsk->connected_table, list);
> >
> > virtio_transport_send_response(vchild, skb);
> >
> > virtio_transport_send_pkt_info()
> >
> > vsock_loopback_send_pkt(VIRTIO_VSOCK_OP_RESPONSE)
> >
> > queue_work(vsock_loopback_work)
> >
> >
> > vsock_loopback_work()
> >
> > virtio_transport_recv_pkt(VIRTIO_VSOCK_OP_RESPONSE)
> > sk =
> > vsock_find_bound_socket(&dst);
> >
> > lock_sock(sk);
> > case
> > TCP_SYN_SENT:
> >
Re: [PATCH net 2/2] vsock/bpf: return early if transport is not assigned
On Wed, Jan 08, 2025 at 07:06:17PM +0100, Stefano Garzarella wrote:
Some of the core functions can only be called if the transport
has been assigned.
As Michal reported, a socket might have the transport at NULL,
for example after a failed connect(), causing the following trace:
BUG: kernel NULL pointer dereference, address: 00a0
#PF: supervisor read access in kernel mode
#PF: error_code(0x) - not-present page
PGD 12faf8067 P4D 12faf8067 PUD 113670067 PMD 0
Oops: Oops: [#1] PREEMPT SMP NOPTI
CPU: 15 UID: 0 PID: 1198 Comm: a.out Not tainted 6.13.0-rc2+
RIP: 0010:vsock_connectible_has_data+0x1f/0x40
Call Trace:
vsock_bpf_recvmsg+0xca/0x5e0
sock_recvmsg+0xb9/0xc0
__sys_recvfrom+0xb3/0x130
__x64_sys_recvfrom+0x20/0x30
do_syscall_64+0x93/0x180
entry_SYSCALL_64_after_hwframe+0x76/0x7e
So we need to check the `vsk->transport` in vsock_bpf_recvmsg(),
especially for connected sockets (stream/seqpacket) as we already
do in __vsock_connectible_recvmsg().
Fixes: 634f1a7110b4 ("vsock: support sockmap")
Reported-by: Michal Luczaj
Closes:
https://lore.kernel.org/netdev/5ca20d4c-1017-49c2-9516-f6f75fd33...@rbox.co/
Signed-off-by: Stefano Garzarella
---
net/vmw_vsock/vsock_bpf.c | 9 +
1 file changed, 9 insertions(+)
diff --git a/net/vmw_vsock/vsock_bpf.c b/net/vmw_vsock/vsock_bpf.c
index 4aa6e74ec295..f201d9eca1df 100644
--- a/net/vmw_vsock/vsock_bpf.c
+++ b/net/vmw_vsock/vsock_bpf.c
@@ -77,6 +77,7 @@ static int vsock_bpf_recvmsg(struct sock *sk, struct msghdr
*msg,
size_t len, int flags, int *addr_len)
{
struct sk_psock *psock;
+ struct vsock_sock *vsk;
int copied;
psock = sk_psock_get(sk);
@@ -84,6 +85,13 @@ static int vsock_bpf_recvmsg(struct sock *sk, struct msghdr
*msg,
return __vsock_recvmsg(sk, msg, len, flags);
lock_sock(sk);
+ vsk = vsock_sk(sk);
+
+ if (!vsk->transport) {
+ copied = -ENODEV;
+ goto out;
+ }
+
if (vsock_has_data(sk, psock) && sk_psock_queue_empty(psock)) {
release_sock(sk);
sk_psock_put(sk, psock);
@@ -108,6 +116,7 @@ static int vsock_bpf_recvmsg(struct sock *sk, struct msghdr
*msg,
copied = sk_msg_recvmsg(sk, psock, msg, len, flags);
}
+out:
release_sock(sk);
sk_psock_put(sk, psock);
--
2.47.1
LGTM!
Reviewed-By: Luigi Leonardi
Re: [PATCH net 2/2] vsock/bpf: return early if transport is not assigned
On Wed, Jan 08, 2025 at 07:06:17PM +0100, Stefano Garzarella wrote:
> Some of the core functions can only be called if the transport
> has been assigned.
>
> As Michal reported, a socket might have the transport at NULL,
> for example after a failed connect(), causing the following trace:
>
> BUG: kernel NULL pointer dereference, address: 00a0
> #PF: supervisor read access in kernel mode
> #PF: error_code(0x) - not-present page
> PGD 12faf8067 P4D 12faf8067 PUD 113670067 PMD 0
> Oops: Oops: [#1] PREEMPT SMP NOPTI
> CPU: 15 UID: 0 PID: 1198 Comm: a.out Not tainted 6.13.0-rc2+
> RIP: 0010:vsock_connectible_has_data+0x1f/0x40
> Call Trace:
> vsock_bpf_recvmsg+0xca/0x5e0
> sock_recvmsg+0xb9/0xc0
> __sys_recvfrom+0xb3/0x130
> __x64_sys_recvfrom+0x20/0x30
> do_syscall_64+0x93/0x180
> entry_SYSCALL_64_after_hwframe+0x76/0x7e
>
> So we need to check the `vsk->transport` in vsock_bpf_recvmsg(),
> especially for connected sockets (stream/seqpacket) as we already
> do in __vsock_connectible_recvmsg().
>
> Fixes: 634f1a7110b4 ("vsock: support sockmap")
> Reported-by: Michal Luczaj
> Closes:
> https://lore.kernel.org/netdev/5ca20d4c-1017-49c2-9516-f6f75fd33...@rbox.co/
> Signed-off-by: Stefano Garzarella
Acked-by: Michael S. Tsirkin
> ---
> net/vmw_vsock/vsock_bpf.c | 9 +
> 1 file changed, 9 insertions(+)
>
> diff --git a/net/vmw_vsock/vsock_bpf.c b/net/vmw_vsock/vsock_bpf.c
> index 4aa6e74ec295..f201d9eca1df 100644
> --- a/net/vmw_vsock/vsock_bpf.c
> +++ b/net/vmw_vsock/vsock_bpf.c
> @@ -77,6 +77,7 @@ static int vsock_bpf_recvmsg(struct sock *sk, struct msghdr
> *msg,
>size_t len, int flags, int *addr_len)
> {
> struct sk_psock *psock;
> + struct vsock_sock *vsk;
> int copied;
>
> psock = sk_psock_get(sk);
> @@ -84,6 +85,13 @@ static int vsock_bpf_recvmsg(struct sock *sk, struct
> msghdr *msg,
> return __vsock_recvmsg(sk, msg, len, flags);
>
> lock_sock(sk);
> + vsk = vsock_sk(sk);
> +
> + if (!vsk->transport) {
> + copied = -ENODEV;
> + goto out;
> + }
> +
> if (vsock_has_data(sk, psock) && sk_psock_queue_empty(psock)) {
> release_sock(sk);
> sk_psock_put(sk, psock);
> @@ -108,6 +116,7 @@ static int vsock_bpf_recvmsg(struct sock *sk, struct
> msghdr *msg,
> copied = sk_msg_recvmsg(sk, psock, msg, len, flags);
> }
>
> +out:
> release_sock(sk);
> sk_psock_put(sk, psock);
>
> --
> 2.47.1
Re: [PATCH v4 4/4] rust: add parameter support to the `module!` macro
On Thu, Jan 09, 2025 at 11:54:59AM +0100, Andreas Hindborg wrote: > This patch includes changes required for Rust kernel modules to utilize > module parameters. This code implements read only support for integer > types without `sysfs` support. I know you want to keep this simple for now, but will you have to go and touch all users of this when you do add the sysfs support later? sysfs wants the mode of the file to be set here, so how do you think of that happening? And don't you need that for your null block driver? Also, what about all the other "types" of module parameters that are currently able to be done, like call-back, hardware control, and unsafe? Are we just not going to do that for rust code (no objection from me, just wanting to be sure.) thanks, greg k-h
Re: [PATCH v4 2/4] rust: str: implement `strip_prefix` for `BStr`
On Thu, Jan 9, 2025 at 11:56 AM Andreas Hindborg wrote: > > Implement `strip_prefix` for `BStr` by deferring to `slice::strip_prefix` > on the underlying `&[u8]`. > > Signed-off-by: Andreas Hindborg Reviewed-by: Alice Ryhl
Re: [PATCH V3 4/8] remoteproc: qcom: add hexagon based WCSS secure PIL driver
On 1/8/2025 9:39 AM, Bjorn Andersson wrote:
> On Tue, Jan 07, 2025 at 03:46:43PM +0530, Gokul Sriram Palanisamy wrote:
>> From: Vignesh Viswanathan
>>
>> Add support to bring up hexagon based WCSS secure PIL remoteproc.
>> IPQ5332, IPQ9574 supports secure PIL remoteproc.
>
> I'd love for this to be extended with a short description of what the
> WCSS secure subsystem is, the reason for a new drivers etc. Following
> the style of
> https://docs.kernel.org/process/submitting-patches.html#describe-your-changes
>
Sure. Bjorn. Will add it here.
>>
>> Signed-off-by: Vignesh Viswanathan
>> Signed-off-by: Manikanta Mylavarapu
>> Signed-off-by: Gokul Sriram Palanisamy
>> ---
>> drivers/remoteproc/Kconfig | 22 ++
>> drivers/remoteproc/Makefile | 1 +
>> drivers/remoteproc/qcom_q6v5_wcss_sec.c | 406
>> 3 files changed, 429 insertions(+)
>> create mode 100644 drivers/remoteproc/qcom_q6v5_wcss_sec.c
>>
>> diff --git a/drivers/remoteproc/Kconfig b/drivers/remoteproc/Kconfig
>> index 83962a114dc9..c4e94b15c538 100644
>> --- a/drivers/remoteproc/Kconfig
>> +++ b/drivers/remoteproc/Kconfig
>> @@ -255,6 +255,28 @@ config QCOM_Q6V5_WCSS
>>Hexagon V5 based WCSS remote processors on e.g. IPQ8074. This is
>>a non-TrustZone wireless subsystem.
>>
>> +config QCOM_Q6V5_WCSS_SEC
>> +tristate "Qualcomm Hexagon based WCSS Secure Peripheral Image Loader"
>> +depends on OF && ARCH_QCOM
>> +depends on QCOM_SMEM
>> +depends on RPMSG_QCOM_SMD || RPMSG_QCOM_SMD=n
>> +depends on RPMSG_QCOM_GLINK_SMEM || RPMSG_QCOM_GLINK_SMEM=n
>> +depends on QCOM_SYSMON || QCOM_SYSMON=n
>> +depends on RPMSG_QCOM_GLINK || RPMSG_QCOM_GLINK=n
>> +depends on QCOM_AOSS_QMP || QCOM_AOSS_QMP=n
>
> Please review these depends, did you inherit a few too many?
sure. Will address.
>
>> +select QCOM_MDT_LOADER
>> +select QCOM_PIL_INFO
>> +select QCOM_Q6V5_COMMON
>> +select QCOM_RPROC_COMMON
>> +select QCOM_SCM
>> +help
>> + Say y here to support the Qualcomm Secure Peripheral Image Loader
>> + for the Hexagon based remote processors on e.g. IPQ5332.
>> +
>> + This is TrustZone wireless subsystem. The firmware is
>> + verified and booted with the help of the Peripheral Authentication
>> + System (PAS) in TrustZone.
>> +
>> config QCOM_SYSMON
>> tristate "Qualcomm sysmon driver"
>> depends on RPMSG
>> diff --git a/drivers/remoteproc/Makefile b/drivers/remoteproc/Makefile
>> index 5ff4e2fee4ab..d4971b672812 100644
>> --- a/drivers/remoteproc/Makefile
>> +++ b/drivers/remoteproc/Makefile
>> @@ -28,6 +28,7 @@ obj-$(CONFIG_QCOM_Q6V5_ADSP) +=
>> qcom_q6v5_adsp.o
>> obj-$(CONFIG_QCOM_Q6V5_MSS) += qcom_q6v5_mss.o
>> obj-$(CONFIG_QCOM_Q6V5_PAS) += qcom_q6v5_pas.o
>> obj-$(CONFIG_QCOM_Q6V5_WCSS)+= qcom_q6v5_wcss.o
>> +obj-$(CONFIG_QCOM_Q6V5_WCSS_SEC)+= qcom_q6v5_wcss_sec.o
>> obj-$(CONFIG_QCOM_SYSMON) += qcom_sysmon.o
>> obj-$(CONFIG_QCOM_WCNSS_PIL)+= qcom_wcnss_pil.o
>> qcom_wcnss_pil-y+= qcom_wcnss.o
>> diff --git a/drivers/remoteproc/qcom_q6v5_wcss_sec.c
>> b/drivers/remoteproc/qcom_q6v5_wcss_sec.c
>> new file mode 100644
>> index ..ef4e893e37c7
>> --- /dev/null
>> +++ b/drivers/remoteproc/qcom_q6v5_wcss_sec.c
>> @@ -0,0 +1,406 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Copyright (C) 2016-2018 Linaro Ltd.
>> + * Copyright (C) 2014 Sony Mobile Communications AB
>> + * Copyright (c) 2012-2018 The Linux Foundation. All rights reserved.
>> + * Copyright (c) 2024-2025 Qualcomm Innovation Center, Inc. All rights
>> reserved.
>> + */
>> +#include
>> +#include
>
> Please check that all these includes are required.
>
sure. will address.
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>
> This will require mailbox maintainer to first accept the tmelcom mailbox
> driver, and share a immutable branch with me (or we have to wait until
> this include file trickles in).
>
> Please ensure that mailbox maintainer is aware of this request.
>
understood.
>> +#include "qcom_common.h"
>> +#include "qcom_q6v5.h"
>> +
sure.
>> +#include "qcom_pil_info.h"
>> +#include "remoteproc_internal.h"
>> +
>> +#define WCSS_CRASH_REASON 421
>> +
>> +#define WCSS_PAS_ID 0x6
>> +#define MPD_WCSS_PAS_ID 0xD
>
> I like lowercase hex digits.
>
>> +
>> +struct wcss_sec {
>> +struct device *dev;
>> +struct qcom_rproc_glink glink_subdev;
>> +struct qcom_rproc_ssr ssr_subdev;
>> +struct qcom_q6v5 q6;
>> +phys_addr_t mem_phys;
>> +phys_addr_t mem_reloc;
>> +void *mem_region;
>> +size_t mem_size;
>> +const struct wcss_data *desc;
>
> Assigned but n
Re: [PATCH net 0/2] bond: fix xfrm offload feature during init
On 1/9/2025 6:17 PM, Hangbin Liu wrote:
On Thu, Jan 09, 2025 at 05:51:07PM +0800, Jianbo Liu wrote:
No, we don't need. But I am trying to understand what you said in your last
email about adding a new lock, or unlocking spin lock in
I *thought* we need the spin lock in xfrm_state_delete(). So to protect
xfrm_state,
But not need in bond_ipsec_del_sa() because the state still hold by
xfrm_state_hold(), right?
Hmm, I'm not sure. If xfrm_state_hold() is safe. Why not just remove the spin
lock in xfrm_state_delete(). This is more straightforward. e.g.
We can't remove the spin lock in xfrm_state_delete(), but I think we can
access the state while holding it, for example, checking (ipsec->xs ==
xs) as you mentioned before, because memory is not freed yet.
diff --git a/net/xfrm/xfrm_state.c b/net/xfrm/xfrm_state.c
index 67ca7ac955a3..150562abf513 100644
--- a/net/xfrm/xfrm_state.c
+++ b/net/xfrm/xfrm_state.c
@@ -784,9 +784,7 @@ int xfrm_state_delete(struct xfrm_state *x)
{
int err;
- spin_lock_bh(&x->lock);
err = __xfrm_state_delete(x);
- spin_unlock_bh(&x->lock);
return err;
}
We can even rename xfrm_state_delete() to xfrm_state_delete() directly.
Thanks
Hangbin
Re: [PATCH] checkpatch: Remove migrated RCU APIs from deprecated_apis
On Thu, Jan 09, 2025 at 07:56:51AM -0800, Joe Perches wrote:
> On Wed, 2025-01-08 at 11:24 -0800, David Reaver wrote:
> > The deprecated_apis map was created in [1] so checkpatch would flag
> > deprecated RCU APIs. These deprecated APIs have since been removed from the
> > kernel. This patch removes them from this map so checkpatch doesn't waste
> > time looking for them, and so readers of checkpatch looking for deprecated
> > APIs don't waste time searching for them.
>
> Acked-by: Joe Perches
>
> Maybe remove the references from rcupdateup.h one day too.
Good point, please see below.
Some instances remain in Documentation/RCU/RTFP.txt, but these are needed
to record the history.
Thanx, Paul
commit a8280286a6425f26785aeedfe9b209a65ca1d6fd
Author: Paul E. McKenney
Date: Thu Jan 9 08:52:15 2025 -0800
rcu: Remove references to old grace-period-wait primitives
The rcu_barrier_sched(), synchronize_sched(), and synchronize_rcu_bh()
RCU API members have been gone for many years. This commit therefore
removes non-historical instances of them.
Reported-by: Joe Perches
Signed-off-by: Paul E. McKenney
diff --git a/Documentation/RCU/rcubarrier.rst b/Documentation/RCU/rcubarrier.rst
index 6da7f66da2a80..12a7b059654f7 100644
--- a/Documentation/RCU/rcubarrier.rst
+++ b/Documentation/RCU/rcubarrier.rst
@@ -329,10 +329,7 @@ Answer:
was first added back in 2005. This is because on_each_cpu()
disables preemption, which acted as an RCU read-side critical
section, thus preventing CPU 0's grace period from completing
- until on_each_cpu() had dealt with all of the CPUs. However,
- with the advent of preemptible RCU, rcu_barrier() no longer
- waited on nonpreemptible regions of code in preemptible kernels,
- that being the job of the new rcu_barrier_sched() function.
+ until on_each_cpu() had dealt with all of the CPUs.
However, with the RCU flavor consolidation around v4.20, this
possibility was once again ruled out, because the consolidated
diff --git a/include/linux/rcupdate.h b/include/linux/rcupdate.h
index 6672e55deeaa4..9b05db8ff0619 100644
--- a/include/linux/rcupdate.h
+++ b/include/linux/rcupdate.h
@@ -800,11 +800,9 @@ do {
\
* sections, invocation of the corresponding RCU callback is deferred
* until after the all the other CPUs exit their critical sections.
*
- * In v5.0 and later kernels, synchronize_rcu() and call_rcu() also
- * wait for regions of code with preemption disabled, including regions of
- * code with interrupts or softirqs disabled. In pre-v5.0 kernels, which
- * define synchronize_sched(), only code enclosed within rcu_read_lock()
- * and rcu_read_unlock() are guaranteed to be waited for.
+ * Both synchronize_rcu() and call_rcu() also wait for regions of code
+ * with preemption disabled, including regions of code with interrupts or
+ * softirqs disabled.
*
* Note, however, that RCU callbacks are permitted to run concurrently
* with new RCU read-side critical sections. One way that this can happen
@@ -859,11 +857,10 @@ static __always_inline void rcu_read_lock(void)
* rcu_read_unlock() - marks the end of an RCU read-side critical section.
*
* In almost all situations, rcu_read_unlock() is immune from deadlock.
- * In recent kernels that have consolidated synchronize_sched() and
- * synchronize_rcu_bh() into synchronize_rcu(), this deadlock immunity
- * also extends to the scheduler's runqueue and priority-inheritance
- * spinlocks, courtesy of the quiescent-state deferral that is carried
- * out when rcu_read_unlock() is invoked with interrupts disabled.
+ * This deadlock immunity also extends to the scheduler's runqueue
+ * and priority-inheritance spinlocks, courtesy of the quiescent-state
+ * deferral that is carried out when rcu_read_unlock() is invoked with
+ * interrupts disabled.
*
* See rcu_read_lock() for more information.
*/
Re: [PATCH] checkpatch: Remove migrated RCU APIs from deprecated_apis
On Wed, 2025-01-08 at 11:24 -0800, David Reaver wrote:
> The deprecated_apis map was created in [1] so checkpatch would flag
> deprecated RCU APIs. These deprecated APIs have since been removed from the
> kernel. This patch removes them from this map so checkpatch doesn't waste
> time looking for them, and so readers of checkpatch looking for deprecated
> APIs don't waste time searching for them.
Acked-by: Joe Perches
Maybe remove the references from rcupdateup.h one day too.
>
> Link:
> https://lore.kernel.org/all/2018192904.3199-13-paul...@linux.ibm.com/ [1]
>
> Signed-off-by: David Reaver
> ---
> scripts/checkpatch.pl | 10 --
> 1 file changed, 10 deletions(-)
>
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index 9eed3683ad76..842a39a01af1 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -834,1 +834,1 @@ foreach my $entry (@mode_permission_funcs) {
> $mode_perms_search = "(?:${mode_perms_search})";
>
> our %deprecated_apis = (
> - "synchronize_rcu_bh"=> "synchronize_rcu",
> - "synchronize_rcu_bh_expedited" => "synchronize_rcu_expedited",
> - "call_rcu_bh" => "call_rcu",
> - "rcu_barrier_bh"=> "rcu_barrier",
> - "synchronize_sched" => "synchronize_rcu",
> - "synchronize_sched_expedited" => "synchronize_rcu_expedited",
> - "call_rcu_sched"=> "call_rcu",
> - "rcu_barrier_sched" => "rcu_barrier",
> - "get_state_synchronize_sched" => "get_state_synchronize_rcu",
> - "cond_synchronize_sched"=> "cond_synchronize_rcu",
> "kmap" => "kmap_local_page",
> "kunmap"=> "kunmap_local",
> "kmap_atomic" => "kmap_local_page",
Re: [PATCH net 1/2] vsock/virtio: discard packets if the transport changes
On 1/9/25 14:42, Stefano Garzarella wrote: > On Thu, Jan 09, 2025 at 02:34:28PM +0100, Michal Luczaj wrote: >> ... >> That said, when I apply this patch, but drop the `sk->sk_state != >> TCP_LISTEN &&`: no more splats. > > We can't drop `sk->sk_state != TCP_LISTEN &&` because listener socket > doesn't have any transport (vsk->transport == NULL), so every connection > request will receive an error, so maybe this is the reason of no splats. Bah, sorry, I didn't run the test suit. > I'm cooking some more patches to fix Hyunwoo's scenario handling better > the close work when the virtio destructor is called. > > I'll run your reproduces to test it, thanks for that! > > Stefano >
Re: [PATCH v4 4/4] rust: add parameter support to the `module!` macro
On Thu, Jan 09, 2025 at 02:03:39PM +0100, Andreas Hindborg wrote: > "Greg KH" writes: > > > On Thu, Jan 09, 2025 at 11:54:59AM +0100, Andreas Hindborg wrote: > >> This patch includes changes required for Rust kernel modules to utilize > >> module parameters. This code implements read only support for integer > >> types without `sysfs` support. > > > > I know you want to keep this simple for now, but will you have to go and > > touch all users of this when you do add the sysfs support later? sysfs > > wants the mode of the file to be set here, so how do you think of that > > happening? > > We would add the required fields to the `module!` macro as optional > fields. No need to touch everyone. Leaving out the sysfs file permission > field would cause the parameter to not show up in sysfs. Ok, that sounds reasonable, thanks! greg k-h
Re: [PATCH net 1/2] vsock/virtio: discard packets if the transport changes
On Thu, Jan 09, 2025 at 11:59:21AM +0100, Stefano Garzarella wrote:
> On Thu, Jan 09, 2025 at 04:13:44AM -0500, Hyunwoo Kim wrote:
> > On Thu, Jan 09, 2025 at 10:01:31AM +0100, Stefano Garzarella wrote:
> > > On Wed, Jan 08, 2025 at 02:31:19PM -0500, Hyunwoo Kim wrote:
> > > > On Wed, Jan 08, 2025 at 07:06:16PM +0100, Stefano Garzarella wrote:
> > > > > If the socket has been de-assigned or assigned to another transport,
> > > > > we must discard any packets received because they are not expected
> > > > > and would cause issues when we access vsk->transport.
> > > > >
> > > > > A possible scenario is described by Hyunwoo Kim in the attached link,
> > > > > where after a first connect() interrupted by a signal, and a second
> > > > > connect() failed, we can find `vsk->transport` at NULL, leading to a
> > > > > NULL pointer dereference.
> > > > >
> > > > > Fixes: c0cfa2d8a788 ("vsock: add multi-transports support")
> > > > > Reported-by: Hyunwoo Kim
> > > > > Reported-by: Wongi Lee
> > > > > Closes:
> > > > > https://lore.kernel.org/netdev/Z2LvdTTQR7dBmPb5@v4bel-B760M-AORUS-ELITE-AX/
> > > > > Signed-off-by: Stefano Garzarella
> > > > > ---
> > > > > net/vmw_vsock/virtio_transport_common.c | 7 +--
> > > > > 1 file changed, 5 insertions(+), 2 deletions(-)
> > > > >
> > > > > diff --git a/net/vmw_vsock/virtio_transport_common.c
> > > > > b/net/vmw_vsock/virtio_transport_common.c
> > > > > index 9acc13ab3f82..51a494b69be8 100644
> > > > > --- a/net/vmw_vsock/virtio_transport_common.c
> > > > > +++ b/net/vmw_vsock/virtio_transport_common.c
> > > > > @@ -1628,8 +1628,11 @@ void virtio_transport_recv_pkt(struct
> > > > > virtio_transport *t,
> > > > >
> > > > > lock_sock(sk);
> > > > >
> > > > > - /* Check if sk has been closed before lock_sock */
> > > > > - if (sock_flag(sk, SOCK_DONE)) {
> > > > > + /* Check if sk has been closed or assigned to another transport
> > > > > before
> > > > > + * lock_sock (note: listener sockets are not assigned to any
> > > > > transport)
> > > > > + */
> > > > > + if (sock_flag(sk, SOCK_DONE) ||
> > > > > + (sk->sk_state != TCP_LISTEN && vsk->transport !=
> > > > > &t->transport)) {
> > > >
> > > > If a race scenario with vsock_listen() is added to the existing
> > > > race scenario, the patch can be bypassed.
> > > >
> > > > In addition to the existing scenario:
> > > > ```
> > > > cpu0
> > > > cpu1
> > > >
> > > > socket(A)
> > > >
> > > > bind(A,
> > > > {cid: VMADDR_CID_LOCAL, port: 1024})
> > > >
> > > > vsock_bind()
> > > >
> > > > listen(A)
> > > >
> > > > vsock_listen()
> > > > socket(B)
> > > >
> > > > connect(B, {cid: VMADDR_CID_LOCAL, port: 1024})
> > > >vsock_connect()
> > > > lock_sock(sk);
> > > > virtio_transport_connect()
> > > >virtio_transport_connect()
> > > > virtio_transport_send_pkt_info()
> > > >vsock_loopback_send_pkt(VIRTIO_VSOCK_OP_REQUEST)
> > > > queue_work(vsock_loopback_work)
> > > > sk->sk_state = TCP_SYN_SENT;
> > > > release_sock(sk);
> > > >
> > > > vsock_loopback_work()
> > > >
> > > > virtio_transport_recv_pkt(VIRTIO_VSOCK_OP_REQUEST)
> > > > sk =
> > > > vsock_find_bound_socket(&dst);
> > > >
> > > > virtio_transport_recv_listen(sk, skb)
> > > >
> > > > child = vsock_create_connected(sk);
> > > >
> > > > vsock_assign_transport()
> > > >
> > > > vvs = kzalloc(sizeof(*vvs), GFP_KERNEL);
> > > >
> > > > vsock_insert_connected(vchild);
> > > >
> > > > list_add(&vsk->connected_table, list);
> > > >
> > > > virtio_transport_send_response(vchild, skb);
> > > >
> > > > virtio_transport_send_pkt_info()
> > > >
> > > > vsock_loopback_send_pkt(VIRTIO_VSOCK_OP_RESPONSE)
> > > >
Re: [syzbot] [virt?] [net?] general protection fault in vsock_connectible_has_data
Hello, syzbot has tested the proposed patch and the reproducer did not trigger any issue: Reported-by: syzbot+3affdbfc986ecd920...@syzkaller.appspotmail.com Tested-by: syzbot+3affdbfc986ecd920...@syzkaller.appspotmail.com Tested on: commit: 767faff2 vsock/bpf: return early if transport is not a.. git tree: https://github.com/stefano-garzarella/linux.git fix-vsock-null-transport console output: https://syzkaller.appspot.com/x/log.txt?x=17c9d4b058 kernel config: https://syzkaller.appspot.com/x/.config?x=4ef22c4fce5135b4 dashboard link: https://syzkaller.appspot.com/bug?extid=3affdbfc986ecd9200fd compiler: Debian clang version 15.0.6, GNU ld (GNU Binutils for Debian) 2.40 Note: no patches were applied. Note: testing is done by a robot and is best-effort only.
Re: [PATCH net 0/2] bond: fix xfrm offload feature during init
On Thu, Jan 09, 2025 at 05:51:07PM +0800, Jianbo Liu wrote:
> > > No, we don't need. But I am trying to understand what you said in your
> > > last
> > > email about adding a new lock, or unlocking spin lock in
> >
> > I *thought* we need the spin lock in xfrm_state_delete(). So to protect
> > xfrm_state,
>
> But not need in bond_ipsec_del_sa() because the state still hold by
> xfrm_state_hold(), right?
Hmm, I'm not sure. If xfrm_state_hold() is safe. Why not just remove the spin
lock in xfrm_state_delete(). This is more straightforward. e.g.
diff --git a/net/xfrm/xfrm_state.c b/net/xfrm/xfrm_state.c
index 67ca7ac955a3..150562abf513 100644
--- a/net/xfrm/xfrm_state.c
+++ b/net/xfrm/xfrm_state.c
@@ -784,9 +784,7 @@ int xfrm_state_delete(struct xfrm_state *x)
{
int err;
- spin_lock_bh(&x->lock);
err = __xfrm_state_delete(x);
- spin_unlock_bh(&x->lock);
return err;
}
We can even rename xfrm_state_delete() to xfrm_state_delete() directly.
Thanks
Hangbin
Re: [PATCH net-next v16 07/26] ovpn: introduce the ovpn_socket object
2025-01-06, 00:27:28 +0100, Antonio Quartulli wrote:
> Hi Sabrina,
>
> On 03/01/2025 18:00, Sabrina Dubroca wrote:
> > Hello Antonio,
> >
> > 2024-12-19, 02:42:01 +0100, Antonio Quartulli wrote:
> > > +static void ovpn_socket_release_kref(struct kref *kref)
> > > + __releases(sock->sock->sk)
> > > +{
> > > + struct ovpn_socket *sock = container_of(kref, struct ovpn_socket,
> > > + refcount);
> > > +
> >
> > [extend with bits of patch 9]
> > > /* UDP sockets are detached in this kref callback because
> > >* we now know for sure that all concurrent users have
> > >* finally gone (refcounter dropped to 0).
> > >*
> > >* Moreover, detachment is performed under lock to prevent
> > >* a concurrent ovpn_socket_new() call with the same socket
> > >* to find the socket still attached but with refcounter 0.
> >
> > I'm not convinced this really works, because ovpn_socket_new() doesn't
> > use the same lock. lock_sock and bh_lock_sock both "lock the socket"
> > in some sense, but they're not mutually exclusive (we talked about
> > that around the TCP patch).
>
> You're right - but what prevents us from always using bh_lock_sock?
TCP detach can sleep, and UDP attach as well (setup_udp_tunnel_sock ->
udp_tunnel_encap_enable -> udp_encap_enable -> static_branch_inc ->
static_key_slow_inc -> cpus_read_lock). UDP detach would also not work
under bh_lock_sock if it really disabled encap on the socket (we end
up in udp_tunnel_encap_enable but that doesn't do anything since encap
is already turned on -- but a "real" detach should disable the encap
and do static_branch_dec).
So attach/detach need to be under lock_sock, not bh_lock_sock.
> > Are you fundamentally opposed to making attach permanent? ie, once
> > a UDP or TCP socket is assigned to an ovpn instance, it can't be
> > detached and reused. I think it would be safer, simpler, and likely
> > sufficient (I don't know openvpn much, but I don't see a use case for
> > moving a socket from one ovpn instance to another, or using it without
> > encap).
>
> I hardly believe a socket will ever be moved to a different instance.
> There is no use case (and no userspace support) for that at the moment.
>
> >
> > Rough idea:
> > - ovpn_socket_new is pretty much unchanged (locking still needed to
> > protect against another simultaneous attach attempt, EALREADY case
> > becomes a bit easier)
> > - ovpn_peer_remove doesn't do anything socket-related
> > - use ->encap_destroy/ovpn_tcp_close() to clean up sk_user_data
> > - no more refcounting on ovpn_socket (since the encap can't be
> > removed, the lifetime to ovpn_socket is tied to its socket)
> >
> > What do you think?
>
> hmm how would that work with UDP?
> On a server all clients may disconnect, but the UDP socket is expected to
> still survive and be re-used for new clients (userspace will keep it alive
> and keep listening for new clients).
>
> Or you're saying that the socket will remain "attached" (i.e. sk_user_data
> set to the ovpn_priv*) even when no more clients are connected?
Yes. Once attached, it stays attached.
> >
> > I'm trying to poke holes into this idea now. close() vs attach worries
> > me a bit.
>
> Can that truly happen?
Actually it can't, so this isn't a concern.
> If a socket is going through close(), there should be some way to mark it as
> "non-attachable".
>
> Actually, do we even need to clean up sk_user_data? The socket is being
> destroyed - why clean that up at all?
If we allocated some memory to store per-socket info, we need to free
it when we detach or close. There's no generic mechanism to free
sk_user_data since the core can't know where it came from, maybe
kfree() isn't appropriate.
--
Sabrina
[PATCH v4 2/4] rust: str: implement `strip_prefix` for `BStr`
Implement `strip_prefix` for `BStr` by deferring to `slice::strip_prefix`
on the underlying `&[u8]`.
Signed-off-by: Andreas Hindborg
---
It is also possible to get this method by implementing
`core::slice::SlicePattern` for `BStr`. `SlicePattern` is unstable, so this
seems more reasonable.
---
rust/kernel/str.rs | 16
1 file changed, 16 insertions(+)
diff --git a/rust/kernel/str.rs b/rust/kernel/str.rs
index
c441acf76ebd1f14919b6d233edfff944619..9c446ff1ad7adba7ca09a5ae9df00fd369a32899
100644
--- a/rust/kernel/str.rs
+++ b/rust/kernel/str.rs
@@ -31,6 +31,22 @@ pub const fn from_bytes(bytes: &[u8]) -> &Self {
// SAFETY: `BStr` is transparent to `[u8]`.
unsafe { &*(bytes as *const [u8] as *const BStr) }
}
+
+/// Strip a prefix from `self`. Delegates to [`slice::strip_prefix`].
+///
+/// # Example
+/// ```
+/// use kernel::b_str;
+/// assert_eq!(Some(b_str!("bar")),
b_str!("foobar").strip_prefix(b_str!("foo")));
+/// assert_eq!(None, b_str!("foobar").strip_prefix(b_str!("bar")));
+/// assert_eq!(Some(b_str!("foobar")),
b_str!("foobar").strip_prefix(b_str!("")));
+/// assert_eq!(Some(b_str!("")),
b_str!("foobar").strip_prefix(b_str!("foobar")));
+/// ```
+pub fn strip_prefix(&self, pattern: &Self) -> Option<&BStr> {
+self.deref()
+.strip_prefix(pattern.deref())
+.map(Self::from_bytes)
+}
}
impl fmt::Display for BStr {
--
2.47.0
Re: [PATCH net 1/2] vsock/virtio: discard packets if the transport changes
On 1/8/25 19:06, Stefano Garzarella wrote:
> If the socket has been de-assigned or assigned to another transport,
> we must discard any packets received because they are not expected
> and would cause issues when we access vsk->transport.
>
> A possible scenario is described by Hyunwoo Kim in the attached link,
> where after a first connect() interrupted by a signal, and a second
> connect() failed, we can find `vsk->transport` at NULL, leading to a
> NULL pointer dereference.
>
> Fixes: c0cfa2d8a788 ("vsock: add multi-transports support")
> Reported-by: Hyunwoo Kim
> Reported-by: Wongi Lee
> Closes:
> https://lore.kernel.org/netdev/Z2LvdTTQR7dBmPb5@v4bel-B760M-AORUS-ELITE-AX/
> Signed-off-by: Stefano Garzarella
> ---
> net/vmw_vsock/virtio_transport_common.c | 7 +--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/net/vmw_vsock/virtio_transport_common.c
> b/net/vmw_vsock/virtio_transport_common.c
> index 9acc13ab3f82..51a494b69be8 100644
> --- a/net/vmw_vsock/virtio_transport_common.c
> +++ b/net/vmw_vsock/virtio_transport_common.c
> @@ -1628,8 +1628,11 @@ void virtio_transport_recv_pkt(struct virtio_transport
> *t,
>
> lock_sock(sk);
>
> - /* Check if sk has been closed before lock_sock */
> - if (sock_flag(sk, SOCK_DONE)) {
> + /* Check if sk has been closed or assigned to another transport before
> + * lock_sock (note: listener sockets are not assigned to any transport)
> + */
> + if (sock_flag(sk, SOCK_DONE) ||
> + (sk->sk_state != TCP_LISTEN && vsk->transport != &t->transport)) {
> (void)virtio_transport_reset_no_sock(t, skb);
> release_sock(sk);
> sock_put(sk);
FWIW, I've tried simplifying Hyunwoo's repro to toy with some tests. Ended
up with
```
from threading import *
from socket import *
from signal import *
def listener(tid):
while True:
s = socket(AF_VSOCK, SOCK_SEQPACKET)
s.bind((1, 1234))
s.listen()
pthread_kill(tid, SIGUSR1)
signal(SIGUSR1, lambda *args: None)
Thread(target=listener, args=[get_ident()]).start()
while True:
c = socket(AF_VSOCK, SOCK_SEQPACKET)
c.connect_ex((1, 1234))
c.connect_ex((42, 1234))
```
which gives me splats with or without this patch.
That said, when I apply this patch, but drop the `sk->sk_state !=
TCP_LISTEN &&`: no more splats.
Re: [PATCH 1/3] selftests/mm: virtual_address_range: Fix error when CommitLimit < 1GiB
On Thu, Jan 09, 2025 at 02:05:43PM +0100, David Hildenbrand wrote:
> >
> > That is clear. The issue would be to figure which chunks are valid to
> > unmap. If something critical like the executable file is unmapped,
> > the process crashes. But see below.
>
> Ah, now I see what you mean. Yes, also the stack etc. will be problematic.
> So IIUC, you want to limit the munmap optimization only to the manually
> mmap()ed parts.
Correct.
> > > > Is it fine to rely on CONFIG_ANON_VMA_NAME?
> > > > That would make it much easier to implement.
> > >
> > > Can you elaborate how you would do it?
> >
> > First set the VMA name after mmap():
> >
> > for (i = 0; i < NR_CHUNKS_LOW; i++) {
> > ptr[i] = mmap(NULL, MAP_CHUNK_SIZE, PROT_READ | PROT_WRITE,
> > MAP_NORESERVE | MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
> >
> > if (ptr[i] == MAP_FAILED) {
> > if (validate_lower_address_hint())
> > ksft_exit_fail_msg("mmap unexpectedly succeeded with
> > hint\n");
> > break;
> > }
> >
> > validate_addr(ptr[i], 0);
> > if (prctl(PR_SET_VMA, PR_SET_VMA_ANON_NAME, ptr[i], MAP_CHUNK_SIZE,
> > "virtual_address_range"))
> > ksft_exit_fail_msg("prctl(PR_SET_VMA_ANON_NAME) failed: %s\n",
> > strerror(errno));
>
> Likely this would prevent merging of VMAs.
>
> With a 1 GiB chunk size, and NR_CHUNKS_LOW == 128TiB, you'd already require
> 128k VMAs. The default limit is frequently 64k.
They are merged for me, as they all share the same name.
PR_SET_VMA(2const) even mentions merging:
Note that assigning an attribute to a virtual memory area might
prevent it from being merged with adjacent virtual memory areas
due to the difference in that attribute's value.
is_mergeable_vma() has an explicit check using anon_vma_name_eq().
> We could just scan the ptr / hptr array to see if this is a manual mmap area
> or not. If this takes too long, one could sort the arrays by address and
> perform a binary search.
>
> Not the most efficient way of doing it, but maybe good enough for this test?
A naive loop is what I tried first, but it took forever.
> Alternatively, store the pointer in a xarray-like tree instead of two
> arrays. Requires a bit more memory ... and we'd have to find a simple
> implementation we could just reuse in this test. So maybe there is a simpler
> way to get it done.
IMO the prctl() is that simpler way.
The only real drawback is the dependency on CONFIG_ANON_VMA_NAME.
We can add an entry to tools/testing/selftests/mm/config for it.
Thomas
Re: [PATCH 0/2] selftest: fix riscv/vector tests
Hello: This series was applied to riscv/linux.git (fixes) by Palmer Dabbelt : On Fri, 20 Dec 2024 17:17:25 +0800 you wrote: > Add test counts and pass message to remove warning of riscv/vector tests. > > Yong-Xuan Wang (2): > tools: selftests: riscv: Add pass message for v_initval_nolibc > tools: selftests: riscv: Add test count for vstate_prctl > > tools/testing/selftests/riscv/vector/v_initval_nolibc.c | 4 > tools/testing/selftests/riscv/vector/vstate_prctl.c | 2 ++ > 2 files changed, 6 insertions(+) Here is the summary with links: - [1/2] tools: selftests: riscv: Add pass message for v_initval_nolibc https://git.kernel.org/riscv/c/503465d4dc40 - [2/2] tools: selftests: riscv: Add test count for vstate_prctl https://git.kernel.org/riscv/c/ebdc22c51ace You are awesome, thank you! -- Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/patchwork/pwbot.html
Re: [PATCH] KVM: selftests: Remove unneeded semicolon
On Tue, 26 Nov 2024 15:37:44 +0800, Chen Ni wrote: > Remove unnecessary semicolons reported by Coccinelle/coccicheck and the > semantic patch at scripts/coccinelle/misc/semicolon.cocci. Applied to kvm-x86 selftests, thanks! [1/1] KVM: selftests: Remove unneeded semicolon https://github.com/kvm-x86/linux/commit/3cd19f150ac6 -- https://github.com/kvm-x86/linux/tree/next
Re: [PATCH] KVM: selftests: Add printf attribute to _no_printf()
On Fri, 13 Dec 2024 14:30:00 -0800, Reinette Chatre wrote: > Annotate the KVM selftests' _no_printf() with the printf format attribute > so that the compiler can help check parameters provided to pr_debug() and > pr_info() irrespective of DEBUG and QUIET being defined. > > [reinette: move attribute right after storage class, rework changelog] > > > [...] Applied to kvm-x86 selftests, thanks! [1/1] KVM: selftests: Add printf attribute to _no_printf() https://github.com/kvm-x86/linux/commit/bd7791078ac2 -- https://github.com/kvm-x86/linux/tree/next
Re: [PATCH v2 0/6] Extend pmu_counters_test to AMD CPUs
On Wed, 18 Sep 2024 20:53:13 +, Colton Lewis wrote: > Extend pmu_counters_test to AMD CPUs. > > As the AMD PMU is quite different from Intel with different events and > feature sets, this series introduces a new code path to test it, > specifically focusing on the core counters including the > PerfCtrExtCore and PerfMonV2 features. Northbridge counters and cache > counters exist, but are not as important and can be deferred to a > later series. > > [...] Applied 1 and a modified version of 2 to kvm-x86 selftests, thanks! [1/6] KVM: x86: selftests: Fix typos in macro variable use https://github.com/kvm-x86/linux/commit/97d0d1655ea8 [2/6] KVM: x86: selftests: Define AMD PMU CPUID leaves https://github.com/kvm-x86/linux/commit/c76a92382805 -- https://github.com/kvm-x86/linux/tree/next
Re: [PATCH v2 0/2] KVM: x86: read the PML log in the same order it was written
On Thu, 19 Dec 2024 17:10:32 -0500, Maxim Levitsky wrote: > Reverse the order in which > the PML log is read to align more closely to the hardware. It should > not affect regular users of the dirty logging but it fixes a unit test > specific assumption in the dirty_log_test dirty-ring mode. > > Best regards, >Maxim Levitsky > > [...] Applied to kvm-x86 vmx, thanks! [1/2] KVM: VMX: refactor PML terminology https://github.com/kvm-x86/linux/commit/ae81ce936ff9 [2/2] KVM: VMX: read the PML log in the same order as it was written https://github.com/kvm-x86/linux/commit/37c3ddfe5238 -- https://github.com/kvm-x86/linux/tree/next
Re: [PATCH v6 12/13] media: i2c: imx214: Fix clock handling on probe error or remove
Hi Sakari,
Am Donnerstag, dem 09.01.2025 um 08:59 + schrieb Sakari Ailus:
> Hi André,
>
> On Wed, Jan 08, 2025 at 10:46:51PM +0100, André Apitzsch wrote:
> > Hi Sakari,
> >
> > Am Mittwoch, dem 08.01.2025 um 11:51 + schrieb Sakari Ailus:
> > > Hi André,
> > >
> > > Thanks for the update.
> > >
> > > On Fri, Dec 20, 2024 at 02:26:11PM +0100, André Apitzsch via B4
> > > Relay
> > > wrote:
> > > > From: André Apitzsch
> > > >
> > > > The driver should disable or unprepare the clock on probe error
> > > > and
> > > > on
> > > > removing the module.
> > > >
> > > > Remove regulator_bulk_disable(), because it is called by
> > > > imx214_power_off().
> > > >
> > > > Acked-by: Ricardo Ribalda
> > > > Signed-off-by: André Apitzsch
> > >
> > > This and the next one appear to be bugfixes. I think it'd be
> > > reasonable to backport them. Any idea where this got broken, for
> > > the
> > > Fixes: tag?
> > > The bug fixed by the last patch was probably introduced with the
> > > driver?
> > >
> >
> > I think both, the missing imx214_power_off() and the wrong link
> > frequency where introduced with
> > > 436190596241 ("media: imx214: Add imx214 camera sensor driver")
> >
> > The first one could also be considered as a fix for the incomplete
> > > 9bc92332cc3f ("media: imx214: Fix the error handling in
> > > imx214_probe()")
>
> This one seems to have missed review. :-(
>
> >
> > Shall I submit a new version with the Fixes: tag?
>
> I checked the fix again and it seems the problem isn't as simple:
> pm_runtime_idle() call leads to powering the device off. It needs to
> either be resumed first or the call to imx214_power_off() be omitted.
>
> I tried to find a good example but the few I found are buggy. :-(
> Getting runtime PM right is just hard. I'll drop the last patch and
> post another to address the problem (plus the intended example
> drivers) if you're fine with that.
>
Fine by me. Go ahead.
Regards,
André
