Re: [PATCH] Fix ospfd segmentation fault on startup
On 20/07/15(Mon) 19:10, Johan Ymerson wrote: On 2015-07-18 16:03:00, Martin Pieuchot wrote: Committed! Thanks and sorry for the delay. Hi! You missed the previous patch Fix ospfd segmentation fault on startup witch prevent ospfd from segfaulting on startup. Without this first patch, ospfd will almost always segfault on startup (instead of just sometime, which it does today). Could you send a single diff for all these issues? Apparently ospf hackers are slacking ;)
Re: [PATCH] Fix ospfd segmentation fault on startup
On Mon, 2015-07-20 at 22:58 +0200, Martin Pieuchot wrote: On 20/07/15(Mon) 19:10, Johan Ymerson wrote: On 2015-07-18 16:03:00, Martin Pieuchot wrote: Committed! Thanks and sorry for the delay. Hi! You missed the previous patch Fix ospfd segmentation fault on startup witch prevent ospfd from segfaulting on startup. Without this first patch, ospfd will almost always segfault on startup (instead of just sometime, which it does today). Could you send a single diff for all these issues? Apparently ospf hackers are slacking ;) Yes, I have it as a single diff. I actually broke it up in two diffs because the two issues are not really related. The second patch only makes the first problem very obvious. It would of course be best if we could make sure we set up event handling (iev_ospfe et al.) before scanning interfaces, but it's kind of a catch 22 here. The event handlers need the interface info to not segfault... So the easy fix was to just check for null pointers in main_imsg_compose_*. /Johan Index: interface.c === RCS file: /cvs/src/usr.sbin/ospfd/interface.c,v retrieving revision 1.75 diff -u -p -r1.75 interface.c --- interface.c 14 May 2012 10:17:21 - 1.75 +++ interface.c 27 May 2015 16:42:51 - @@ -338,8 +338,10 @@ if_act_start(struct iface *iface) struct in_addr addr; struct timeval now; - if (!((iface-flags IFF_UP) - LINK_STATE_IS_UP(iface-linkstate))) + if (!(iface-flags IFF_UP) || + (!LINK_STATE_IS_UP(iface-linkstate) + !(iface-media_type == IFT_CARP + iface-linkstate == LINK_STATE_DOWN))) return (0); if (iface-media_type == IFT_CARP iface-passive == 0) { Index: kroute.c === RCS file: /cvs/src/usr.sbin/ospfd/kroute.c,v retrieving revision 1.98 diff -u -p -r1.98 kroute.c --- kroute.c11 Feb 2015 05:57:44 - 1.98 +++ kroute.c27 May 2015 16:42:51 - @@ -1019,6 +1019,9 @@ if_change(u_short ifindex, int flags, st return; } + /* notify ospfe about interface link state */ + main_imsg_compose_ospfe(IMSG_IFINFO, 0, kif, sizeof(struct kif)); + reachable = (kif-flags IFF_UP) LINK_STATE_IS_UP(kif-link_state); @@ -1026,9 +1029,6 @@ if_change(u_short ifindex, int flags, st return; /* nothing changed wrt nexthop validity */ kif-nh_reachable = reachable; - - /* notify ospfe about interface link state */ - main_imsg_compose_ospfe(IMSG_IFINFO, 0, kif, sizeof(struct kif)); /* update redistribute list */ RB_FOREACH(kr, kroute_tree, krt) { Index: ospfd.c === RCS file: /cvs/src/usr.sbin/ospfd/ospfd.c,v retrieving revision 1.83 diff -u -p -r1.83 ospfd.c --- ospfd.c 10 Feb 2015 05:24:48 - 1.83 +++ ospfd.c 27 May 2015 16:42:51 - @@ -511,13 +511,15 @@ main_dispatch_rde(int fd, short event, v void main_imsg_compose_ospfe(int type, pid_t pid, void *data, u_int16_t datalen) { - imsg_compose_event(iev_ospfe, type, 0, pid, -1, data, datalen); + if (iev_ospfe) + imsg_compose_event(iev_ospfe, type, 0, pid, -1, data, datalen); } void main_imsg_compose_rde(int type, pid_t pid, void *data, u_int16_t datalen) { - imsg_compose_event(iev_rde, type, 0, pid, -1, data, datalen); + if (iev_rde) + imsg_compose_event(iev_rde, type, 0, pid, -1, data, datalen); } void
Re: [PATCH] Fix ospfd segmentation fault on startup
On 2015-07-18 16:03:00, Martin Pieuchot wrote: Committed! Thanks and sorry for the delay. Hi! You missed the previous patch Fix ospfd segmentation fault on startup witch prevent ospfd from segfaulting on startup. Without this first patch, ospfd will almost always segfault on startup (instead of just sometime, which it does today). /Johan
Re: [PATCH] Fix ospfd segmentation fault on startup
On Mon, Jul 20, 2015 at 09:32:20PM +, Johan Ymerson wrote: On Mon, 2015-07-20 at 22:58 +0200, Martin Pieuchot wrote: On 20/07/15(Mon) 19:10, Johan Ymerson wrote: On 2015-07-18 16:03:00, Martin Pieuchot wrote: Committed! Thanks and sorry for the delay. Hi! You missed the previous patch Fix ospfd segmentation fault on startup witch prevent ospfd from segfaulting on startup. Without this first patch, ospfd will almost always segfault on startup (instead of just sometime, which it does today). Could you send a single diff for all these issues? Apparently ospf hackers are slacking ;) Yes, I have it as a single diff. I actually broke it up in two diffs because the two issues are not really related. The second patch only makes the first problem very obvious. It would of course be best if we could make sure we set up event handling (iev_ospfe et al.) before scanning interfaces, but it's kind of a catch 22 here. The event handlers need the interface info to not segfault... So the easy fix was to just check for null pointers in main_imsg_compose_*. /Johan Yes, lets go with this version. OK claudio@ Index: interface.c === RCS file: /cvs/src/usr.sbin/ospfd/interface.c,v retrieving revision 1.75 diff -u -p -r1.75 interface.c --- interface.c 14 May 2012 10:17:21 - 1.75 +++ interface.c 27 May 2015 16:42:51 - @@ -338,8 +338,10 @@ if_act_start(struct iface *iface) struct in_addr addr; struct timeval now; - if (!((iface-flags IFF_UP) - LINK_STATE_IS_UP(iface-linkstate))) + if (!(iface-flags IFF_UP) || + (!LINK_STATE_IS_UP(iface-linkstate) + !(iface-media_type == IFT_CARP + iface-linkstate == LINK_STATE_DOWN))) return (0); if (iface-media_type == IFT_CARP iface-passive == 0) { Index: kroute.c === RCS file: /cvs/src/usr.sbin/ospfd/kroute.c,v retrieving revision 1.98 diff -u -p -r1.98 kroute.c --- kroute.c11 Feb 2015 05:57:44 - 1.98 +++ kroute.c27 May 2015 16:42:51 - @@ -1019,6 +1019,9 @@ if_change(u_short ifindex, int flags, st return; } + /* notify ospfe about interface link state */ + main_imsg_compose_ospfe(IMSG_IFINFO, 0, kif, sizeof(struct kif)); + reachable = (kif-flags IFF_UP) LINK_STATE_IS_UP(kif-link_state); @@ -1026,9 +1029,6 @@ if_change(u_short ifindex, int flags, st return; /* nothing changed wrt nexthop validity */ kif-nh_reachable = reachable; - - /* notify ospfe about interface link state */ - main_imsg_compose_ospfe(IMSG_IFINFO, 0, kif, sizeof(struct kif)); /* update redistribute list */ RB_FOREACH(kr, kroute_tree, krt) { Index: ospfd.c === RCS file: /cvs/src/usr.sbin/ospfd/ospfd.c,v retrieving revision 1.83 diff -u -p -r1.83 ospfd.c --- ospfd.c 10 Feb 2015 05:24:48 - 1.83 +++ ospfd.c 27 May 2015 16:42:51 - @@ -511,13 +511,15 @@ main_dispatch_rde(int fd, short event, v void main_imsg_compose_ospfe(int type, pid_t pid, void *data, u_int16_t datalen) { - imsg_compose_event(iev_ospfe, type, 0, pid, -1, data, datalen); + if (iev_ospfe) + imsg_compose_event(iev_ospfe, type, 0, pid, -1, data, datalen); } void main_imsg_compose_rde(int type, pid_t pid, void *data, u_int16_t datalen) { - imsg_compose_event(iev_rde, type, 0, pid, -1, data, datalen); + if (iev_rde) + imsg_compose_event(iev_rde, type, 0, pid, -1, data, datalen); } void -- :wq Claudio
Re: [PATCH] Fix ospfd segmentation fault on startup
Johan Ymerson(johan.ymer...@transmode.com) on 2015.07.20 21:32:20 +: On Mon, 2015-07-20 at 22:58 +0200, Martin Pieuchot wrote: On 20/07/15(Mon) 19:10, Johan Ymerson wrote: On 2015-07-18 16:03:00, Martin Pieuchot wrote: Committed! Thanks and sorry for the delay. Hi! You missed the previous patch Fix ospfd segmentation fault on startup witch prevent ospfd from segfaulting on startup. Without this first patch, ospfd will almost always segfault on startup (instead of just sometime, which it does today). Could you send a single diff for all these issues? Apparently ospf hackers are slacking ;) Yes, I have it as a single diff. I actually broke it up in two diffs because the two issues are not really related. The second patch only makes the first problem very obvious. It would of course be best if we could make sure we set up event handling (iev_ospfe et al.) before scanning interfaces, but it's kind of a catch 22 here. The event handlers need the interface info to not segfault... So the easy fix was to just check for null pointers in main_imsg_compose_*. /Johan fixed it. ok benno@ Index: interface.c === RCS file: /cvs/src/usr.sbin/ospfd/interface.c,v retrieving revision 1.75 diff -u -p -r1.75 interface.c --- interface.c 14 May 2012 10:17:21 - 1.75 +++ interface.c 27 May 2015 16:42:51 - @@ -338,8 +338,10 @@ if_act_start(struct iface *iface) struct in_addr addr; struct timeval now; - if (!((iface-flags IFF_UP) - LINK_STATE_IS_UP(iface-linkstate))) + if (!(iface-flags IFF_UP) || + (!LINK_STATE_IS_UP(iface-linkstate) + !(iface-media_type == IFT_CARP + iface-linkstate == LINK_STATE_DOWN))) return (0); if (iface-media_type == IFT_CARP iface-passive == 0) { Index: kroute.c === RCS file: /cvs/src/usr.sbin/ospfd/kroute.c,v retrieving revision 1.98 diff -u -p -r1.98 kroute.c --- kroute.c11 Feb 2015 05:57:44 - 1.98 +++ kroute.c27 May 2015 16:42:51 - @@ -1019,6 +1019,9 @@ if_change(u_short ifindex, int flags, st return; } + /* notify ospfe about interface link state */ + main_imsg_compose_ospfe(IMSG_IFINFO, 0, kif, sizeof(struct kif)); + reachable = (kif-flags IFF_UP) LINK_STATE_IS_UP(kif-link_state); @@ -1026,9 +1029,6 @@ if_change(u_short ifindex, int flags, st return; /* nothing changed wrt nexthop validity */ kif-nh_reachable = reachable; - - /* notify ospfe about interface link state */ - main_imsg_compose_ospfe(IMSG_IFINFO, 0, kif, sizeof(struct kif)); /* update redistribute list */ RB_FOREACH(kr, kroute_tree, krt) { Index: ospfd.c === RCS file: /cvs/src/usr.sbin/ospfd/ospfd.c,v retrieving revision 1.83 diff -u -p -r1.83 ospfd.c --- ospfd.c 10 Feb 2015 05:24:48 - 1.83 +++ ospfd.c 27 May 2015 16:42:51 - @@ -511,13 +511,15 @@ main_dispatch_rde(int fd, short event, v void main_imsg_compose_ospfe(int type, pid_t pid, void *data, u_int16_t datalen) { - imsg_compose_event(iev_ospfe, type, 0, pid, -1, data, datalen); + if (iev_ospfe) + imsg_compose_event(iev_ospfe, type, 0, pid, -1, data, datalen); } void main_imsg_compose_rde(int type, pid_t pid, void *data, u_int16_t datalen) { - imsg_compose_event(iev_rde, type, 0, pid, -1, data, datalen); + if (iev_rde) + imsg_compose_event(iev_rde, type, 0, pid, -1, data, datalen); } void --
Re: [PATCH] Fix ospfd segmentation fault on startup
commited, thx for your diff. /Benno Johan Ymerson(johan.ymer...@transmode.com) on 2015.07.20 21:32:20 +: On Mon, 2015-07-20 at 22:58 +0200, Martin Pieuchot wrote: On 20/07/15(Mon) 19:10, Johan Ymerson wrote: On 2015-07-18 16:03:00, Martin Pieuchot wrote: Committed! Thanks and sorry for the delay. Hi! You missed the previous patch Fix ospfd segmentation fault on startup witch prevent ospfd from segfaulting on startup. Without this first patch, ospfd will almost always segfault on startup (instead of just sometime, which it does today). Could you send a single diff for all these issues? Apparently ospf hackers are slacking ;) Yes, I have it as a single diff. I actually broke it up in two diffs because the two issues are not really related. The second patch only makes the first problem very obvious. It would of course be best if we could make sure we set up event handling (iev_ospfe et al.) before scanning interfaces, but it's kind of a catch 22 here. The event handlers need the interface info to not segfault... So the easy fix was to just check for null pointers in main_imsg_compose_*. /Johan Index: interface.c === RCS file: /cvs/src/usr.sbin/ospfd/interface.c,v retrieving revision 1.75 diff -u -p -r1.75 interface.c --- interface.c 14 May 2012 10:17:21 - 1.75 +++ interface.c 27 May 2015 16:42:51 - @@ -338,8 +338,10 @@ if_act_start(struct iface *iface) struct in_addr addr; struct timeval now; - if (!((iface-flags IFF_UP) - LINK_STATE_IS_UP(iface-linkstate))) + if (!(iface-flags IFF_UP) || + (!LINK_STATE_IS_UP(iface-linkstate) + !(iface-media_type == IFT_CARP + iface-linkstate == LINK_STATE_DOWN))) return (0); if (iface-media_type == IFT_CARP iface-passive == 0) { Index: kroute.c === RCS file: /cvs/src/usr.sbin/ospfd/kroute.c,v retrieving revision 1.98 diff -u -p -r1.98 kroute.c --- kroute.c11 Feb 2015 05:57:44 - 1.98 +++ kroute.c27 May 2015 16:42:51 - @@ -1019,6 +1019,9 @@ if_change(u_short ifindex, int flags, st return; } + /* notify ospfe about interface link state */ + main_imsg_compose_ospfe(IMSG_IFINFO, 0, kif, sizeof(struct kif)); + reachable = (kif-flags IFF_UP) LINK_STATE_IS_UP(kif-link_state); @@ -1026,9 +1029,6 @@ if_change(u_short ifindex, int flags, st return; /* nothing changed wrt nexthop validity */ kif-nh_reachable = reachable; - - /* notify ospfe about interface link state */ - main_imsg_compose_ospfe(IMSG_IFINFO, 0, kif, sizeof(struct kif)); /* update redistribute list */ RB_FOREACH(kr, kroute_tree, krt) { Index: ospfd.c === RCS file: /cvs/src/usr.sbin/ospfd/ospfd.c,v retrieving revision 1.83 diff -u -p -r1.83 ospfd.c --- ospfd.c 10 Feb 2015 05:24:48 - 1.83 +++ ospfd.c 27 May 2015 16:42:51 - @@ -511,13 +511,15 @@ main_dispatch_rde(int fd, short event, v void main_imsg_compose_ospfe(int type, pid_t pid, void *data, u_int16_t datalen) { - imsg_compose_event(iev_ospfe, type, 0, pid, -1, data, datalen); + if (iev_ospfe) + imsg_compose_event(iev_ospfe, type, 0, pid, -1, data, datalen); } void main_imsg_compose_rde(int type, pid_t pid, void *data, u_int16_t datalen) { - imsg_compose_event(iev_rde, type, 0, pid, -1, data, datalen); + if (iev_rde) + imsg_compose_event(iev_rde, type, 0, pid, -1, data, datalen); } void --