I would indeed very much appreciate if you could dig further, because at the moment I am not convinced that this patch is addressing the root cause of the issue, as opposed to hiding it.
Hi Kevin,
Apologies it has taken me so long to respond to this. I thought it better to continue this discussion on this thread rather than create a new patch submission. I have found the source of the issue from the iptables side. There are several other failing test cases in the iptables tests which are fixed by this patch. These are as follows, with the corresponding dmesg output:
././testcases/ipt-save/0001loa d-dumps_0 ip_tables: TCPMSS.0 target: invalid size 8 (kernel) != (user) 16 ././testcases/ipt-save/0002load-fedora27-firewalld_0 ip_tables: CHECKSUM.0 target: invalid size 8 (kernel) != (user) 16 ././testcases/nft-only/0008-basechain-policy_0 ip_tables: CT.2 target: invalid size 72 (kernel) != (user) 80
Taking this last testcase, we can see that the CT.2 target corresponds to this xt_ct_target_info_v1 struct [1], which does indeed contain a pointer/capability (I have no idea what happens when you have that __attribute__((aligned(8)) on a pointer in the morello compiler though? I guess it is ignored or changed to 16?). The size is determined using the macro XT_ALIGN(sizeof(struct xt_ct_target_info_v1)), which gives 72 in non-purecap, and 80 in purecap.
In the example we originally discussed before, using the SNAT.0 xt_target struct, the situation is a little more complex. We see that when the netlink message is composed by iptables, this function append_range is called [2], which increases the size of the netlink attribute to be put into the message. In this function the sizeof(*info) is what causes the discrepancy between purecap and regular aarch64. *info is this struct below, which has size 64B in purecap rather than 56B in non-purecap, due to the need for 16B alignment.:
struct ipt_natinfo { struct xt_entry_target t; [3] struct nf_nat_ipv4_multi_range_compat mr; [4] };
Now the issue that I see in this example above when using the patch first proposed, is that this appends a redundant 8B of data to the end of the netlink attribute. I think that this particular problem could be fixed from the iptables side. However, it does not change the situation with the CT.2 target, and since developers are free to create their own module extensions to netfilter/iptables by adding their own matches/targets, I think that the XT_ALIGN fix I originally proposed would be necessary as long as the XT_ALIGN macro remains in the uapi and is able to be used by userspace tools. I don't know what you reckon?
Many thanks,
Josh
[1] https://elixir.bootlin.com/linux/latest/source/include/uapi/linux/netfilter/... [2] https://git.netfilter.org/iptables/tree/extensions/libipt_SNAT.c?h=v1.8.7#n5... [3] https://git.netfilter.org/iptables/tree/include/linux/netfilter/x_tables.h?h... [4] https://git.netfilter.org/iptables/tree/include/linux/netfilter/nf_nat.h?h=v...
On Thu, 4 Apr 2024 at 10:37, Kevin Brodsky kevin.brodsky@arm.com wrote:
On 04/04/2024 11:12, josh lant wrote:
Hi Kevin,
Thanks for your input here. I appreciate you taking the time to go through it bit by bit...
I guess I need to do some more digging. So should I not submit a v2 for this patch until I am able to have a more concrete answer about the cause of the issue? Or should I do that regardless, the update with just the void __user *, and not submitting the defconfig patch above at all (I only found 1 overlapping setting between the standard arm64 config and my updated one)?
I would indeed very much appreciate if you could dig further, because at the moment I am not convinced that this patch is addressing the root cause of the issue, as opposed to hiding it. If you manage to confirm that patch 2 is the appropriate solution, please feel free to send a v2 (with an updated commit message). It is probably best to drop patch 1.
Thanks again for taking the time to investigate this!
Kevin
PS: one small thing, pure plain text emails are preferred when replying on mailing lists - it is possible to toggle this in the Gmail web UI (and classic email clients like Thunderbird).
Cheers,
Josh