Re: alignment issue using sctp_assoc_stats via getsockopt

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Em 08-10-2015 11:07, Neil Horman escreveu:
On Thu, Oct 08, 2015 at 04:06:59PM +0200, Daniel Borkmann wrote:
On 10/08/2015 03:45 PM, Neil Horman wrote:
On Thu, Oct 08, 2015 at 08:26:56AM -0300, Marcelo wrote:
Em 8 de outubro de 2015 08:11:40 BRT, Neil Horman <nhorman@xxxxxxxxxxxxx> escreveu:
On Wed, Oct 07, 2015 at 06:11:55PM -0300, Marcelo Ricardo Leitner
wrote:
Hi Jakub,

On Tue, Oct 06, 2015 at 01:04:43PM +0200, Jakub Tomalak wrote:
Greetings,

I have discovered an alignment issue when using SCTP in a 32-bit
binary on a 64-bit host.

Essentially, the problem is that struct sctp_assoc_stats contains a
sockaddr_storage field and is at a different offset for 32-bit and
64-bit architectures (well, Intel/AMD anyway).

Essentially when a 32-bit binary makes a getsockopt call with
sctp_assoc_stats againts a 64-bit kernel, the kernel copies the
data
in with a different alignment, causing the 32-bit binary to receive
incorrect data for all of the chunk counters (fields after the
sockaddr field)..

The fix is relatively simple for i386 and x86_64 architectures. I
am
not sure about MIPS64 and ARM, since a colleague mentioned they do
not
support unaligned memory access and also mentioned that MIPS64
doesn't
allow alignment with word sizes smaller than 8 bytes.

Here is the diff I tested against both 32-bit and 64-bit RHEL and
Ubuntu systems (with kernels ranging from 3.12.x to 3.16.y).

diff --git a/include/uapi/linux/sctp.h b/include/uapi/linux/sctp.h
index ce70fe6..41aa566 100644
--- a/include/uapi/linux/sctp.h
+++ b/include/uapi/linux/sctp.h
@@ -873,7 +873,7 @@ struct sctp_assoc_stats {
         __u64           sas_iodchunks;   /* Ordered data chunks
received */
         __u64           sas_octrlchunks; /* Control chunks sent */
         __u64           sas_ictrlchunks; /* Control chunks received
*/
-};
+} __attribute__((packed, aligned(4)));

/*
   * 8.1 sctp_bindx()
@@ -900,6 +900,6 @@ struct sctp_paddrthlds {
         struct sockaddr_storage spt_address;
         __u16 spt_pathmaxrxt;
         __u16 spt_pathpfthld;
-};
+} __attribute__((packed, aligned(4)));

#endif /* _UAPI_SCTP_H */

I have no idea what the official process is for submitting patches
to
the SCTP module, and since this is probably a very edge case
scenario,
I thought I would at least mention it in case anyone else comes
across
similar problems.

We can say that it starts by sharing your thoughts like you did. And
when you are confident with a patch, you can submit it to netdev@
directly while keeping this ML Cc'ed. Dave Miller is the one who
integrates sctp patches.

Now about your patch. I think it's just too late to fix that.  That
issue is exposed to user-space for too long so that I'm afraid it has
already become a "feature". If we fix that, it will break
applications
that are handling that in some other way.

   Marcelo

--
For the life of me I can't recall how to do this, but can we add a
thunk layer
to the syscall to detect the arch of the calling process and fix up the
alignment while inside the syscall, and translate back in the epiloge?

Neil

Add this to lksctp-tools you mean?


No, to the kernel.  IIRC in the early days of x86_64, lots of syscalls had
problem like this, where 32 bit user space and 64 bit kernels didn't agree on
word sizes and alignment, so there was a thunk layer that detected that
condition, made a copy of the user passed data, fixed the alignment, and gave
that to the real syscall in the kernel, then reversed the process before
returning to user space.  It was horrid and messy, but it seems like we could do
something simmilar here

But for such things we have the compat infrastructure in the kernel, f.e.
see ffd5939381c6 ("net: sctp: fix sctp_connectx abi for ia32 emulation/compat
mode"). So, I think something similar for the getsockopt() might be required.

Yes, thank you Daniel, thats what I was thinking of, not thunking

That still changes the ABI and will break applications that circumvented that, but yeah we have a case of success, I hope Dave will accept a similar fix for this one too. ;-)

  Marcelo

--
To unsubscribe from this list: send the line "unsubscribe linux-sctp" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Networking Development]     [Linux OMAP]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux