Hello Andreas Greve! Big thanks for debugging this problem. Seems like there are always new problems with this feature popping up and I was about to disable it in Debian soon if noone showed interest in getting it fixed (again).
Some more comments inline below... On Wed, May 07, 2014 at 12:49:44AM +0200, Andreas Greve wrote: > Hallo, > > I had the same problems with -j MARK > > and found the reason and a solution for me. > > The reason for the segfault is the iptables ABI change form 5 to 6 > in 2011-04-14. The iptables package maintainer in Debian has split out the xtables library and added proper versioning to the packaging, so now we shouldn't have random breakages because of the iproute2 module being linked against the wrong library atleast... Still we need to have more people tracking the actual API changes and modifying the upstream iproute2 source code to work with the new xtables modifications..... > in file extensions/libxt_MARK.c the struct field .parse is not > initialized any more. They use now the fields .x6_parse etc. So this > change breaks wheezy stable iproute tc/m_xt.c in function > parse_ipt(...) at every placer where ->parse(...) is used. > This bug is fixed in iproute2 (2013-01-16 16:14:48 act_ipt fix > xtables breakage) upstream/3.8.0. > I tested it for libxt_MARK.c with source (3.8.0) fetched from > kernel.org and it works. > > It works with debian/3.14.0-1 for me too. > My system is a little bit special ( xen-4.4 with a kernel 3.12.17 in dom0 ). FWIW, "zobel" has recently showed interest in maintaining a backported iproute2 package for those running Wheezy. See https://lists.debian.org/debian-backports/2014/04/msg00138.html (I think this even got uploaded...) > > But there is an other segfault bug in iproute tc/m_xt.c > print_ipt(...) which ist not fixed up to debian/3.14.0-1 > > > in print_ipt(...) xtables will be initialized with the static > struct tcipt_globals > at xtables_init_all(&tcipt_globals, ....) and at the end the call > xtables_free_opts(1) will destroy the .opts field of this static > struct. This will cause a segfault in tc filter show ... if there > exists more than one filter -J MARK action (for example). Thanks for tracking this down (and providing all these details about it)! > > I found a fix but I am not sure if it is correct. > /* tcipt_globals must be cloned because xtables_free_opts(1) destroy > tcipt_globals */ > > print_ipt(....) { > .... > struct xtables_globals tmp_tcipt_globals; > memcpy(&tmp_tcipt_globals, &tcipt_globals, sizeof(struct xtables_globals) ); > > replace all other occurrence of tcipt_globals with > tmp_tcipt_globals in print_ipt(....) I've only quickly looked at this now and it makes me wonder if a more simple change would also solve this: Simply add: tcipt_globals.opts = tcipt_globals.orig_opts; After each call to: xtables_free_opts(1); This should as far as I can see restore the struct to it's original glory instead of leaving the "opts" field set to NULL (which is what xtables_free_opts will do to it). Hopefully this should avoid having to duplicate the entire struct each time. For extra bonus points, ask the iptables/xtables developers why they set it to NULL instead of orig_opts when they're done. (They already have checks in xtables_free_opts to make sure to not free opts if opts == orig_opts.) > > Up to now I have tested this fix only with 3.8.0 after testing with > 3.14 I will make a bug report at the iproute2 comunity Here are some administrative details and suggestions for how to submit your changes to upstream: clone the upstream git repository, details at: https://git.kernel.org/cgit/linux/kernel/git/shemminger/iproute2.git/ Commit your changes, write a short summary on first line, then empty line, then longer description of the problem. Export your patch(es) using git format-patch: git format-patch -o /tmp -s origin/master.. The "-s" will add a "Signed-off-by" line for you which means you just signed off on the "Developer Certificate of origin" (for more info see http://elinux.org/Developer_Certificate_Of_Origin ). Look over your patches and once they look nice and tidy, send them to the upstream maintainer: Stephen Hemminger <[email protected]> Also CC the developer mailing list: [email protected] > > Sorry for the rough English. I hope the Information helps. No problem! Your input is very much appreciated! Regards, Andreas Henriksson -- To UNSUBSCRIBE, email to [email protected] with a subject of "unsubscribe". Trouble? Contact [email protected]
