Source-Changes-D archive
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]
Re: CVS commit: src/sys/kern
On 2018/08/02 18:10, Kamil Rytarowski wrote:
On 02.08.2018 06:33, Masanobu SAITOH wrote:
On 2018/08/02 13:28, SAITOH Masanobu wrote:
Module Name: src
Committed By: msaitoh
Date: Thu Aug 2 04:28:56 UTC 2018
Modified Files:
src/sys/kern: uipc_mbuf2.c
Log Message:
Adjust alignment in m_pulldown().
IP6_EXTHDR_GET() and M_REGION_GET() do m_pulldown(). When
m_pulldown() copies
data into M_TRAILINGSPACE, the alignment might be changed. There are a
lot of
IP6_EXTHDR_GET() calls, so I think it's not good to check the
alignment after
every IP6_EXTHDR_GET() call. This change fixes this problem in
m_pulldown().
In this commit, the next mbuf are 4 byte aligned. For networking, I've
never
heard that 64bit alignment is required, so I think it would be OK.
I don't know this is the best solution, but it's better than nothing.
OK'd by maxv@.
After committing this change, the workaround code for PR#50776 can
be removed.
Not PR#50776 but PR#50766
(panic in tcp_input.c on the banana pi (earm7hf) when trying to connect
an ipv6 address through a gif ipv6 in ipv4 tunnel)
I've identified various similar misalignment in the kernel.
I will land the utility (micro-UBSan) to the sources soon (this week?).
It got delayed few days because rebase to HEAD with newer clang
introduced new checks and I needed to cover them with handlers and tests.
The problem is that many people (include me) didn't know
IP6_EXTHDR_GET() and M_REGION_GET() (and m_pulldown()) might change
the next mbuf's alignment. It's not documented.
Currently,
grep _ALIGNED_P */*.h
netinet/icmp_private.h:#define ICMP_HDR_ALIGNED_P(ic) 1
netinet/icmp_private.h:#define ICMP_HDR_ALIGNED_P(ic) ((((vaddr_t) (ic)) & 3) == 0)
netinet/igmp_var.h:#define IGMP_HDR_ALIGNED_P(ig) 1
netinet/igmp_var.h:#define IGMP_HDR_ALIGNED_P(ig) ((((vaddr_t) (ig)) & 3) == 0)
netinet/ip_private.h:#define IP_HDR_ALIGNED_P(ip) 1
netinet/ip_private.h:#define IP_HDR_ALIGNED_P(ip) ((((vaddr_t) (ip)) & 3) == 0)
netinet/tcp_private.h:#define TCP_HDR_ALIGNED_P(th) 1
netinet/tcp_private.h:#define TCP_HDR_ALIGNED_P(th) ((((vaddr_t)(th)) & 3) == 0)
netinet/udp_private.h:#define UDP_HDR_ALIGNED_P(uh) 1
netinet/udp_private.h:#define UDP_HDR_ALIGNED_P(uh) ((((vaddr_t) (uh)) & 3) == 0)
netinet6/ip6_private.h:#define IP6_HDR_ALIGNED_P(ip) 1
netinet6/ip6_private.h:#define IP6_HDR_ALIGNED_P(ip) ((((vaddr_t) (ip)) & 3) == 0)
so 4 byte alignment is OK. I heard that OpenFlow's packet has uint64_t values,
so this solution might not work in future.
Correct solution is to check all code around IP6_EXTHDR_GET(),
M_REGION_GET() and m_pulldown(). If a code before those call expects
the mbuf is aligned, (re-)check the alignment with xxx_HDR_ALIGNED_P().
If it's not aligned, (re-)align with m_copyup().
28 IP6_EXTHDR_GET()s:
https://nxr.netbsd.org/search?q=IP6_EXTHDR_GET&project=src&defs=&refs=&path=&hist=
12 M_REGION_GET()s:
https://nxr.netbsd.org/search?q=M_REGION_GET&project=src&defs=&refs=&path=&hist=
21 m_pulldown()s:
https://nxr.netbsd.org/search?q=m_pulldown&project=src&defs=&refs=&path=&hist=
Not so much?
Let me know if my understanding is incorrect.
To generate a diff of this commit:
cvs rdiff -u -r1.32 -r1.33 src/sys/kern/uipc_mbuf2.c
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
--
-----------------------------------------------
SAITOH Masanobu (msaitoh%execsw.org@localhost
msaitoh%netbsd.org@localhost)
Home |
Main Index |
Thread Index |
Old Index