Bug#765577: (no subject)
Control: tag -1 pending
Hey Faidon,
Faidon Liambotis [2015-04-22 15:25 +0300]:
-new_rule_pattern=$(echo ^SUBSYSTEM==\net\, ACTION==\add\$match | sed
-re 's/([\?\*])/\\\1/g')
+new_rule_pattern=$(echo ^SUBSYSTEM==\net\, ACTION==\add\$match | sed
-re 's/([\?\*\{\}])/\\\1/g')
Thanks for spotting this! I tested this now with Marco's test script,
and it still works fine. Committed to master now:
http://anonscm.debian.org/cgit/pkg-systemd/systemd.git/commit/?id=3d02f64154
(to experimental too).
I have to say... constructing regexps in shell is tricky and the
whole escaping-with-sed logic feels like a hack. I think a literal
grep (i.e. -F) would be better here, especially since I don't see
the point of an exact match (even if the file was modified by the
sysadmin, the right thing would to not write a new rule anyway).
This is probably something to be considered post-jessie.
Yeah, the whole generator is a horrible thing.. For the future I
actually want to propose a bolder change:
https://lists.debian.org/debian-devel/2015/05/msg00170.html
Martin
--
Martin Pitt| http://www.piware.de
Ubuntu Developer (www.ubuntu.com) | Debian Developer (www.debian.org)
signature.asc
Description: Digital signature
___
Pkg-systemd-maintainers mailing list
Pkg-systemd-maintainers@lists.alioth.debian.org
http://lists.alioth.debian.org/cgi-bin/mailman/listinfo/pkg-systemd-maintainers
Processed: Re: Bug#765577: (no subject)
Processing control commands: tag -1 pending Bug #765577 [udev-udeb] netboot install writes duplicates to 70-persistent-net.rules Bug #777126 [udev-udeb] udev: duplicate eth? entries Added tag(s) pending. Added tag(s) pending. -- 765577: http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=765577 777126: http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=777126 Debian Bug Tracking System Contact ow...@bugs.debian.org with problems ___ Pkg-systemd-maintainers mailing list Pkg-systemd-maintainers@lists.alioth.debian.org http://lists.alioth.debian.org/cgi-bin/mailman/listinfo/pkg-systemd-maintainers
Bug#765577: (no subject)
reopen 765577 !
found 765577 215-14
thanks
On Mon, Mar 30, 2015 at 06:06:47AM +0200, Marco d'Itri wrote:
I see that we have independently devised the same fix, I am attaching
a test case and a more refined version of your patch.
I tried Jessie RC3 today and immediately found that the fix is,
unfortunately, buggy. Your patch constructs a regexp and takes care to
escape metacharacters ? and * with a sed but does not escape { and
} that are also metacharacters in the extended set of POSIX regexps.
These are always found in the string-to-be-matched here with
'ATTR{dev_id}==0x0' and 'ATTR{type}==1', so the if always fails.
This was likely not caught by your test case (and was harder to debug
and figure out!) because GNU grep's -E mode handles { as both a literal
and a metacharacter heuristically for historic reasons (consult grep's
manpage for that) but busybox grep does not:
$ echo 'foo{bar}' test
$ egrep 'foo{bar}' test
foo{bar}
$ busybox egrep 'foo{bar}' test
egrep: bad regex 'foo{bar}'
$ egrep 'fo{1,2}' test
foo{bar}
$ busybox egrep 'fo{1,2}' test
foo{bar}
Note that this is NOT a bug in busybox; foo{bar} is indeed an invalid
extended POSIX regexp and busybox is right to complain and error out.
The very minimal last-minute fix below did the trick for me but I have
to say... constructing regexps in shell is tricky and the whole
escaping-with-sed logic feels like a hack. I think a literal grep (i.e.
-F) would be better here, especially since I don't see the point of an
exact match (even if the file was modified by the sysadmin, the right
thing would to not write a new rule anyway). This is probably something
to be considered post-jessie.
Thanks,
Faidon
diff --git a/debian/extra/write_net_rules b/debian/extra/write_net_rules
index 38a3ca0..fedc0f1 100644
--- a/debian/extra/write_net_rules
+++ b/debian/extra/write_net_rules
@@ -118,7 +118,7 @@ basename=${INTERFACE%%[0-9]*}
match=$match, KERNEL==\$basename*\
# build a regular expression that matches the new rule that we want to write
-new_rule_pattern=$(echo ^SUBSYSTEM==\net\, ACTION==\add\$match | sed -re
's/([\?\*])/\\\1/g')
+new_rule_pattern=$(echo ^SUBSYSTEM==\net\, ACTION==\add\$match | sed -re
's/([\?\*\{\}])/\\\1/g')
# Double check if the new rule has already been written. This happens if
# multiple add events are generated before the script returns and udevd
___
Pkg-systemd-maintainers mailing list
Pkg-systemd-maintainers@lists.alioth.debian.org
http://lists.alioth.debian.org/cgi-bin/mailman/listinfo/pkg-systemd-maintainers
Processed: Re: Bug#765577: (no subject)
Processing commands for cont...@bugs.debian.org:
reopen 765577 !
Bug #765577 {Done: Martin Pitt mp...@debian.org} [udev-udeb] netboot install
writes duplicates to 70-persistent-net.rules
Bug #777126 {Done: Martin Pitt mp...@debian.org} [udev-udeb] udev: duplicate
eth? entries
'reopen' may be inappropriate when a bug has been closed with a version;
all fixed versions will be cleared, and you may need to re-add them.
Bug reopened
Changed Bug submitter to 'Faidon Liambotis parav...@debian.org' from 'Petter
Reinholdtsen p...@hungry.com'
No longer marked as fixed in versions systemd/219-6 and systemd/215-14.
No longer marked as fixed in versions systemd/219-6 and systemd/215-14.
found 765577 215-14
Bug #765577 [udev-udeb] netboot install writes duplicates to
70-persistent-net.rules
Bug #777126 [udev-udeb] udev: duplicate eth? entries
Marked as found in versions systemd/215-14.
Marked as found in versions systemd/215-14.
thanks
Stopping processing here.
Please contact me if you need assistance.
--
765577: http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=765577
777126: http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=777126
Debian Bug Tracking System
Contact ow...@bugs.debian.org with problems
___
Pkg-systemd-maintainers mailing list
Pkg-systemd-maintainers@lists.alioth.debian.org
http://lists.alioth.debian.org/cgi-bin/mailman/listinfo/pkg-systemd-maintainers
Bug#765577: (no subject)
On Mar 18, Faidon Liambotis parav...@debian.org wrote:
Well, the root cause IMO is that 75-persistent-net-generator.rules is
inherently susceptible to races. It's my understanding that it's valid
for events to be triggered multiple times -- there are multiple places
in d-i that udevadm trigger is called, including start-udev (as
shipped by udev-udeb) which would trigger another set of add events
beyond the original hotplug one.
I do not think that this is valid in the sense that the kernel could
generate multiple add events, but removing all misuses of udevadm
trigger requires some work and may not even be possible at this time.
I see that we have independently devised the same fix, I am attaching
a test case and a more refined version of your patch.
--
ciao,
Marco
#!/bin/sh -e
rm -f /etc/udev/rules.d/70-persistent-net.rules
rmmod e1000e || true
modprobe e1000e
sleep 1
rm -f /etc/udev/rules.d/70-persistent-net.rules
iteration=0
while : ; do
iteration=$((iteration + 1))
if ! echo add /sys/class/net/eth0/uevent; then
echo Failed at iteration $iteration!
exit
fi
# do not send too many events to udev because they will all be queued
# and eventually processed
if [ $iteration -eq 2500 ]; then
echo Aborting at iteration $iteration!
exit
fi
done
--- /lib/udev/write_net_rules.orig 2015-03-30 05:18:43.228147201 +0200
+++ /lib/udev/write_net_rules 2015-03-30 06:00:39.181085432 +0200
@@ -117,6 +117,18 @@
basename=${INTERFACE%%[0-9]*}
match=$match, KERNEL==\$basename*\
+# build a regular expression that matches the new rule that we want to write
+new_rule_pattern=$(echo ^SUBSYSTEM==\net\, ACTION==\add\$match | sed -re 's/([\?\*])/\\\1/g')
+
+# Double check if the new rule has already been written. This happens if
+# multiple add events are generated before the script returns and udevd
+# renames the interfaces. See #765577 for details.
+if egrep -q $new_rule_pattern \
+$RO_RULES_FILE $([ -e $RULES_FILE ] echo $RULES_FILE); then
+ unlock_rules_file
+ exit 0
+fi
+
if [ $INTERFACE_NAME ]; then
# external tools may request a custom name
COMMENT=$COMMENT (custom name provided by external tool)
pgp0CXb73md6m.pgp
Description: PGP signature
___
Pkg-systemd-maintainers mailing list
Pkg-systemd-maintainers@lists.alioth.debian.org
http://lists.alioth.debian.org/cgi-bin/mailman/listinfo/pkg-systemd-maintainers
