| Message ID | 20220624141831.1069403-1-i.maximets@ovn.org |
|---|---|
| State | Accepted |
| Commit | 334d43bc0349687b847cc89dd74e54b3fb0f500c |
| Headers | show |
| Series | [ovs-dev] packets: Fix misaligned write to MPLS lse. | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
| ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
| ovsrobot/intel-ovs-compilation | success | test: success |
On 24 Jun 2022, at 16:18, Ilya Maximets wrote: > MPLS header is only 2 byte aligned, so the value has to be written > in parts. Also, even though the 'struct mpls_hdr' has only one > field, it's cleaner to not access that field directly. > > lib/packets.c:432:9: runtime error: > store to misaligned address 0x61b000756382 for type 'ovs_be32' > (aka 'unsigned int'), which requires 4 byte alignment > 0x61b000756382: note: pointer points here > 00 00 be be be be be be ff ff ff ff ff ff a6 36 77 20 ... > ^ > 0 0xbb30ae in add_mpls lib/packets.c:432:17 > 1 0x9934d2 in odp_execute_actions lib/odp-execute.c:1072:17 > 2 0x830946 in dp_netdev_execute_actions lib/dpif-netdev.c:9106:5 > 3 0x830946 in handle_packet_upcall lib/dpif-netdev.c:8294:5 > 4 0x82ea5e in fast_path_processing lib/dpif-netdev.c:8390:25 > 5 0x7ed87f in dp_netdev_input__ lib/dpif-netdev.c:8479:9 > 6 0x7eb5fc in dp_netdev_input lib/dpif-netdev.c:8517:5 > 7 0x81dada in dp_netdev_process_rxq_port lib/dpif-netdev.c:5329:19 > 8 0x7f0063 in dpif_netdev_run lib/dpif-netdev.c:6664:25 > 9 0x85f036 in dpif_run lib/dpif.c:467:16 > 10 0x61833a in type_run ofproto/ofproto-dpif.c:366:9 > 11 0x5c210e in ofproto_type_run ofproto/ofproto.c:1822:31 > 12 0x565db2 in bridge_run__ vswitchd/bridge.c:3245:9 > 13 0x562f82 in bridge_run vswitchd/bridge.c:3310:5 > 14 0x59a98c in main vswitchd/ovs-vswitchd.c:129:9 > 15 0x7ff895c3acf2 in __libc_start_main (/lib64/libc.so.6+0x3acf2) > 16 0x47e60d in _start (vswitchd/ovs-vswitchd+0x47e60d) > > Fixes: 1917ace89364 ("Encap & Decap actions for MPLS packet type.") > Signed-off-by: Ilya Maximets <i.maximets@ovn.org> Changes look good to me, and tests are passing. Acked-by: Eelco Chaudron <echaudro@redhat.com>
On 6/24/22 16:36, Eelco Chaudron wrote: > > > On 24 Jun 2022, at 16:18, Ilya Maximets wrote: > >> MPLS header is only 2 byte aligned, so the value has to be written >> in parts. Also, even though the 'struct mpls_hdr' has only one >> field, it's cleaner to not access that field directly. >> >> lib/packets.c:432:9: runtime error: >> store to misaligned address 0x61b000756382 for type 'ovs_be32' >> (aka 'unsigned int'), which requires 4 byte alignment >> 0x61b000756382: note: pointer points here >> 00 00 be be be be be be ff ff ff ff ff ff a6 36 77 20 ... >> ^ >> 0 0xbb30ae in add_mpls lib/packets.c:432:17 >> 1 0x9934d2 in odp_execute_actions lib/odp-execute.c:1072:17 >> 2 0x830946 in dp_netdev_execute_actions lib/dpif-netdev.c:9106:5 >> 3 0x830946 in handle_packet_upcall lib/dpif-netdev.c:8294:5 >> 4 0x82ea5e in fast_path_processing lib/dpif-netdev.c:8390:25 >> 5 0x7ed87f in dp_netdev_input__ lib/dpif-netdev.c:8479:9 >> 6 0x7eb5fc in dp_netdev_input lib/dpif-netdev.c:8517:5 >> 7 0x81dada in dp_netdev_process_rxq_port lib/dpif-netdev.c:5329:19 >> 8 0x7f0063 in dpif_netdev_run lib/dpif-netdev.c:6664:25 >> 9 0x85f036 in dpif_run lib/dpif.c:467:16 >> 10 0x61833a in type_run ofproto/ofproto-dpif.c:366:9 >> 11 0x5c210e in ofproto_type_run ofproto/ofproto.c:1822:31 >> 12 0x565db2 in bridge_run__ vswitchd/bridge.c:3245:9 >> 13 0x562f82 in bridge_run vswitchd/bridge.c:3310:5 >> 14 0x59a98c in main vswitchd/ovs-vswitchd.c:129:9 >> 15 0x7ff895c3acf2 in __libc_start_main (/lib64/libc.so.6+0x3acf2) >> 16 0x47e60d in _start (vswitchd/ovs-vswitchd+0x47e60d) >> >> Fixes: 1917ace89364 ("Encap & Decap actions for MPLS packet type.") >> Signed-off-by: Ilya Maximets <i.maximets@ovn.org> > > > Changes look good to me, and tests are passing. > > Acked-by: Eelco Chaudron <echaudro@redhat.com> > Thanks! Applied to master and branch-2.17. Best regards, Ilya Maximets.
diff --git a/lib/packets.c b/lib/packets.c index d0fba8176..874066e3c 100644 --- a/lib/packets.c +++ b/lib/packets.c @@ -427,9 +427,9 @@ add_mpls(struct dp_packet *packet, ovs_be16 ethtype, ovs_be32 lse, } if (!l3_encap) { - ovs_be32 *header = dp_packet_push_uninit(packet, MPLS_HLEN); + struct mpls_hdr *header = dp_packet_push_uninit(packet, MPLS_HLEN); - *header = lse; + put_16aligned_be32(&header->mpls_lse, lse); packet->l2_5_ofs = 0; packet->packet_type = PACKET_TYPE_BE(OFPHTN_ETHERTYPE, ntohs(ethtype));
MPLS header is only 2 byte aligned, so the value has to be written in parts. Also, even though the 'struct mpls_hdr' has only one field, it's cleaner to not access that field directly. lib/packets.c:432:9: runtime error: store to misaligned address 0x61b000756382 for type 'ovs_be32' (aka 'unsigned int'), which requires 4 byte alignment 0x61b000756382: note: pointer points here 00 00 be be be be be be ff ff ff ff ff ff a6 36 77 20 ... ^ 0 0xbb30ae in add_mpls lib/packets.c:432:17 1 0x9934d2 in odp_execute_actions lib/odp-execute.c:1072:17 2 0x830946 in dp_netdev_execute_actions lib/dpif-netdev.c:9106:5 3 0x830946 in handle_packet_upcall lib/dpif-netdev.c:8294:5 4 0x82ea5e in fast_path_processing lib/dpif-netdev.c:8390:25 5 0x7ed87f in dp_netdev_input__ lib/dpif-netdev.c:8479:9 6 0x7eb5fc in dp_netdev_input lib/dpif-netdev.c:8517:5 7 0x81dada in dp_netdev_process_rxq_port lib/dpif-netdev.c:5329:19 8 0x7f0063 in dpif_netdev_run lib/dpif-netdev.c:6664:25 9 0x85f036 in dpif_run lib/dpif.c:467:16 10 0x61833a in type_run ofproto/ofproto-dpif.c:366:9 11 0x5c210e in ofproto_type_run ofproto/ofproto.c:1822:31 12 0x565db2 in bridge_run__ vswitchd/bridge.c:3245:9 13 0x562f82 in bridge_run vswitchd/bridge.c:3310:5 14 0x59a98c in main vswitchd/ovs-vswitchd.c:129:9 15 0x7ff895c3acf2 in __libc_start_main (/lib64/libc.so.6+0x3acf2) 16 0x47e60d in _start (vswitchd/ovs-vswitchd+0x47e60d) Fixes: 1917ace89364 ("Encap & Decap actions for MPLS packet type.") Signed-off-by: Ilya Maximets <i.maximets@ovn.org> --- lib/packets.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)