diff mbox series

[lsm/dev] netfilter: Use correct length value in ctnetlink_secctx_size

Message ID 97463c75-2edd-47e0-b081-a235891bee6e@schaufler-ca.com (mailing list archive)
State Under Review
Delegated to: Paul Moore
Headers show
Series [lsm/dev] netfilter: Use correct length value in ctnetlink_secctx_size | expand

Commit Message

Casey Schaufler Nov. 1, 2024, 6:43 p.m. UTC
Use the correct value for the context length returned by
security_secid_to_secctx().

Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
---
 net/netfilter/nf_conntrack_netlink.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Paul Moore Nov. 1, 2024, 8:07 p.m. UTC | #1
On Fri, Nov 1, 2024 at 2:43 PM Casey Schaufler <casey@schaufler-ca.com> wrote:
>
> Use the correct value for the context length returned by
> security_secid_to_secctx().
>
> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
> ---
>  net/netfilter/nf_conntrack_netlink.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

Thanks Casey, I'm going to merge this into lsm/dev-staging for
testing, but additional comments, reviews, etc. are always welcome.
Paul Moore Nov. 1, 2024, 10:35 p.m. UTC | #2
On Fri, Nov 1, 2024 at 4:07 PM Paul Moore <paul@paul-moore.com> wrote:
>
> On Fri, Nov 1, 2024 at 2:43 PM Casey Schaufler <casey@schaufler-ca.com> wrote:
> >
> > Use the correct value for the context length returned by
> > security_secid_to_secctx().
> >
> > Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
> > ---
> >  net/netfilter/nf_conntrack_netlink.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
>
> Thanks Casey, I'm going to merge this into lsm/dev-staging for
> testing, but additional comments, reviews, etc. are always welcome.

Unfortunately it looks like there is still an issue.  Running the NFS
tests from the selinux-testsuite I hit the panic splat below.

There are instructions on how to run the selinux-testsuite's NFS tests
in the test suite's README.md (linked below), but basically make sure
you have a working test suite, install the "nfs-utils" package, and
then run `./tools/nfs.sh`.

https://github.com/SELinuxProject/selinux-testsuite

If it helps, the kernel I used for testing can be found in the
directory below, it's version
6.12.0-0.rc5.20241101git6c52d4da.48.3.secnext.fc42 on x86_64.  There
are debuginfos in there too if needed.

https://repo.paul-moore.com/rawhide/x86_64

[   36.233175] NFSD: starting 90-second grace period (net f0000000)
[   36.348290] Key type dns_resolver registered
[   36.538794] NFS: Registering the id_resolver key type
[   36.539855] Key type id_resolver registered
[   36.540109] Key type id_legacy registered
[   36.563308] NFSD: all clients done reclaiming, ending NFSv4 grace
period (net f0000000)
[   36.718416] BUG: kernel NULL pointer dereference, address: 0000000000000000
[   36.724501] #PF: supervisor read access in kernel mode
[   36.724824] #PF: error_code(0x0000) - not-present page
[   36.725126] PGD 0 P4D 0
[   36.725285] Oops: Oops: 0000 [#1] PREEMPT SMP PTI
[   36.725588] CPU: 2 UID: 0 PID: 1228 Comm: sh Not tainted
6.12.0-0.rc5.20241101git6c52d4da.48.3.secnext.fc42.x86_64 #1
[   36.726284] Hardware name: Red Hat KVM, BIOS 0.5.1 01/01/2011
[   36.726638] RIP: 0010:memcpy_orig+0x1e/0x140
[   36.726922] Code: 90 90 90 90 90 90 90 90 90 90 90 90 66 0f 1f 00
48 89 f8 48 83 fa 20 0f 82 86 00 00 00 40 38 fe 7c 35 48 83 ea 20 48
83 ea 20 <4c> 8b 06 4c 8b 4e 08 4c 8b 56 10 4c 8b 5e 18 48 8d 76 20 4c
89 07
[   36.727992] RSP: 0018:ffffb9e680aa7900 EFLAGS: 00010283
[   36.728298] RAX: ffff95cac1458800 RBX: ffff95cac5035000 RCX: 0000000000000000
[   36.728734] RDX: ffffffffffffffe6 RSI: 0000000000000000 RDI: ffff95cac1458800
[   36.729148] RBP: ffff95cad8f1be40 R08: 0000000000000800 R09: 0000000000000000
[   36.729572] R10: ffffb9e680aa78e8 R11: 0000000000000192 R12: ffff95cac0a85e00
[   36.730075] R13: ffff95cac09f0a80 R14: ffffb9e680aa7ad0 R15: ffff95caf189b400
[   36.730494] FS:  00007f961aa98740(0000) GS:ffff95cbf7d00000(0000)
knlGS:0000000000000000
[   36.730964] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[   36.731300] CR2: 0000000000000000 CR3: 00000001197b8000 CR4: 00000000001506f0
[   36.731720] Call Trace:
[   36.731875]  <TASK>
[   36.732010]  ? __die_body.cold+0x19/0x27
[   36.732247]  ? page_fault_oops+0x15a/0x2f0
[   36.732490]  ? search_module_extables+0x19/0x60
[   36.732766]  ? search_bpf_extables+0x5f/0x80
[   36.733024]  ? exc_page_fault+0x7e/0x180
[   36.733261]  ? asm_exc_page_fault+0x26/0x30
[   36.733511]  ? memcpy_orig+0x1e/0x140
[   36.733842]  nfs4_opendata_alloc+0x314/0x470 [nfsv4]
[   36.734431]  nfs4_do_open+0x207/0xd10 [nfsv4]
[   36.734957]  ? security_sid_to_context_core+0x114/0x150
[   36.735492]  ? selinux_dentry_init_security+0xc5/0x120
[   36.736061]  nfs4_atomic_open+0xbf/0x140 [nfsv4]
[   36.736638]  nfs_atomic_open+0x20f/0x670 [nfs]
[   36.737205]  path_openat+0xbfa/0x12e0
[   36.737626]  do_filp_open+0xc4/0x170
[   36.738063]  do_sys_openat2+0xae/0xe0
[   36.738489]  __x64_sys_openat+0x55/0xa0
[   36.738939]  do_syscall_64+0x82/0x160
[   36.739368]  ? __count_memcg_events+0x77/0x130
[   36.739859]  ? count_memcg_events.constprop.0+0x1a/0x30
[   36.740285]  ? handle_mm_fault+0x21b/0x330
[   36.740622]  ? do_user_addr_fault+0x55a/0x7b0
[   36.740992]  ? exc_page_fault+0x7e/0x180
[   36.741316]  entry_SYSCALL_64_after_hwframe+0x76/0x7e
[   36.741739] RIP: 0033:0x7f961ab072d6
[   36.742056] Code: 89 df e8 8d c1 00 00 8b 93 08 03 00 00 59 5e 48
83 f8 fc 75 15 83 e2 39 83 fa 08 75 0d e8 32 ff ff ff 66 90 48 8b 45
10 0f 05 <48> 8b 5d f8 c9 c3 0f 1f 40 00 f3 0f 1e fa 55 48 89 e5 48 83
ec 08
[   36.743559] RSP: 002b:00007ffcce132c80 EFLAGS: 00000202 ORIG_RAX:
0000000000000101
[   36.744172] RAX: ffffffffffffffda RBX: 00007f961aa98740 RCX: 00007f961ab072d6
[   36.744748] RDX: 0000000000000241 RSI: 000055a76c121be0 RDI: ffffffffffffff9c
[   36.745323] RBP: 00007ffcce132c90 R08: 0000000000000000 R09: 0000000000000000
[   36.745899] R10: 00000000000001b6 R11: 0000000000000202 R12: 0000000000000000
[   36.746527] R13: 0000000000000001 R14: 000055a76c121be0 R15: 000055a7308f67c0
[   36.747187]  </TASK>
[   36.747388] Modules linked in: rpcsec_gss_krb5 nfsv4 dns_resolver
nfsd auth_rpcgss nfsv3 nfs_acl nfs lockd grace netfs rfkill vfat fat
intel_rapl_msr intel_rapl_common virtio_balloon i2c_piix4 virtio_net
i2c_smbus net_failover failover joydev loop nfnetlink vsock_loopback
vmw_vsock_virtio_transport_common vmw_vsock_vmci_transport vsock
vmw_vmci rpcrdma sunrpc rdma_ucm ib_srpt ib_isert iscsi_target_mod
target_core_mod ib_iser libiscsi scsi_transport_iscsi ib_umad rdma_cm
ib_ipoib iw_cm ib_cm fuse mlx5_ib macsec ib_uverbs ib_core
crct10dif_pclmul crc32_pclmul crc32c_intel polyval_clmulni
polyval_generic ghash_clmulni_intel virtio_blk sha512_ssse3
sha256_ssse3 sha1_ssse3 ata_generic pata_acpi serio_raw mlx5_core
mlxfw psample tls pci_hyperv_intf qemu_fw_cfg virtio_console
[   36.753333] CR2: 0000000000000000
[   36.753832] ---[ end trace 0000000000000000 ]---
[   36.754298] RIP: 0010:memcpy_orig+0x1e/0x140
[   36.756227] Code: 90 90 90 90 90 90 90 90 90 90 90 90 66 0f 1f 00
48 89 f8 48 83 fa 20 0f 82 86 00 00 00 40 38 fe 7c 35 48 83 ea 20 48
83 ea 20 <4c> 8b 06 4c 8b 4e 08 4c 8b 56 10 4c 8b 5e 18 48 8d 76 20 4c
89 07
[   36.757911] RSP: 0018:ffffb9e680aa7900 EFLAGS: 00010283
[   36.758560] RAX: ffff95cac1458800 RBX: ffff95cac5035000 RCX: 0000000000000000
[   36.759869] RDX: ffffffffffffffe6 RSI: 0000000000000000 RDI: ffff95cac1458800
[   36.760455] RBP: ffff95cad8f1be40 R08: 0000000000000800 R09: 0000000000000000
[   36.761039] R10: ffffb9e680aa78e8 R11: 0000000000000192 R12: ffff95cac0a85e00
[   36.761718] R13: ffff95cac09f0a80 R14: ffffb9e680aa7ad0 R15: ffff95caf189b400
[   36.762316] FS:  00007f961aa98740(0000) GS:ffff95cbf7d00000(0000)
knlGS:0000000000000000
[   36.762990] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[   36.763476] CR2: 0000000000000000 CR3: 00000001197b8000 CR4: 00000000001506f0
Paul Moore Nov. 1, 2024, 10:37 p.m. UTC | #3
On Fri, Nov 1, 2024 at 6:35 PM Paul Moore <paul@paul-moore.com> wrote:
> On Fri, Nov 1, 2024 at 4:07 PM Paul Moore <paul@paul-moore.com> wrote:
> > On Fri, Nov 1, 2024 at 2:43 PM Casey Schaufler <casey@schaufler-ca.com> wrote:
> > >
> > > Use the correct value for the context length returned by
> > > security_secid_to_secctx().
> > >
> > > Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
> > > ---
> > >  net/netfilter/nf_conntrack_netlink.c | 4 ++--
> > >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > Thanks Casey, I'm going to merge this into lsm/dev-staging for
> > testing, but additional comments, reviews, etc. are always welcome.
>
> Unfortunately it looks like there is still an issue.  Running the NFS
> tests from the selinux-testsuite I hit the panic splat below ...

To be clear, this is from code in the lsm/dev-staging branch, not
lsm/dev or lsm/next so while we need to get this fixed, it isn't a "uh
oh, we broke linux-next" type of situation.
diff mbox series

Patch

diff --git a/net/netfilter/nf_conntrack_netlink.c b/net/netfilter/nf_conntrack_netlink.c
index dd74d4c67c69..edf08cc89f17 100644
--- a/net/netfilter/nf_conntrack_netlink.c
+++ b/net/netfilter/nf_conntrack_netlink.c
@@ -663,14 +663,14 @@  static inline size_t ctnetlink_acct_size(const struct nf_conn *ct)
 static inline int ctnetlink_secctx_size(const struct nf_conn *ct)
 {
 #ifdef CONFIG_NF_CONNTRACK_SECMARK
-	int len, ret;
+	int ret;
 
 	ret = security_secid_to_secctx(ct->secmark, NULL);
 	if (ret < 0)
 		return 0;
 
 	return nla_total_size(0) /* CTA_SECCTX */
-	       + nla_total_size(sizeof(char) * len); /* CTA_SECCTX_NAME */
+	       + nla_total_size(sizeof(char) * ret); /* CTA_SECCTX_NAME */
 #else
 	return 0;
 #endif