ebtables lock file fcntl errno value not correctly checked
| Affects | Status | Importance | Assigned to | Milestone | |
|---|---|---|---|---|---|
| ebtables (Ubuntu) |
Fix Released
|
Medium
|
Dan Streetman | ||
| Trusty |
Fix Released
|
Medium
|
Dan Streetman | ||
| Xenial |
Fix Released
|
Medium
|
Dan Streetman | ||
| Artful |
Fix Released
|
Medium
|
Dan Streetman | ||
| Bionic |
Fix Released
|
Medium
|
Dan Streetman | ||
| Cosmic |
Fix Released
|
Medium
|
Dan Streetman | ||
Bug Description
[impact]
bug 1645324 introduced code to improve the existing file-based locking, by using fcntl instead of exclusive file opening.
However, when fcntl fails the new code tries to check for errno of EAGAIN or EACCES, but does so incorrectly:
+ ret = fcntl(lockfd, F_SETLK, &fl);
+ if (ret == -1 && errno != (EAGAIN || EACCES))
"errno != (EAGAIN || EACCES)" is not correct, because it will always evaluate to true when errno is either EAGAIN or EACCES; the bitwise OR of EAGAIN (11) and EACCES (13) produces 15 (ENOTBLK) which will never match either, so != always is true.
[test case]
run ebtables in a tight loop from two separate shells, to force lockfile contention, for example:
#!/bin/bash
dev="$1"
while test 1; do
sleep 0
ebtables --concurrent -t nat -A PREROUTING -i ${dev} -j ACCEPT
if [ $? -ne 0 ]; then
echo "odd!"
fi
ebtables --concurrent -t nat -D PREROUTING -i ${dev} -j ACCEPT
if [ $? -ne 0 ]; then
echo "odd!"
fi
done
that can be run with any text param (e.g. "foo" and "bar") from 2 shells, to produce the failure:
Unable to create lock file /var/lib/
[regression potential]
like the previous patch, this change also has the potential to introduce errors in file locking that ebtables uses; however considering the file locking currently does not work at all due to the above logic error, this patch should only help. The only regression potential is see is the possibility of breaking non-contended file locking, which currently does work.
[other info]
see previous bug 1645324
https:/
| Dan Streetman (ddstreet) wrote | #1 |
| Changed in ebtables (Ubuntu): | |
| status: | New → In Progress |
| assignee: | nobody → Dan Streetman (ddstreet) |
| importance: | Undecided → Medium |
| no longer affects: | ebtables (Ubuntu Precise) |
| no longer affects: | ebtables (Ubuntu Precise) |
| Changed in ebtables (Ubuntu Trusty): | |
| status: | New → In Progress |
| Changed in ebtables (Ubuntu Bionic): | |
| status: | New → In Progress |
| Changed in ebtables (Ubuntu Artful): | |
| status: | New → In Progress |
| Changed in ebtables (Ubuntu Xenial): | |
| status: | New → In Progress |
| Changed in ebtables (Ubuntu Bionic): | |
| importance: | Undecided → Medium |
| Changed in ebtables (Ubuntu Artful): | |
| importance: | Undecided → Medium |
| Changed in ebtables (Ubuntu Xenial): | |
| importance: | Undecided → Medium |
| Changed in ebtables (Ubuntu Trusty): | |
| importance: | Undecided → Medium |
| Changed in ebtables (Ubuntu Bionic): | |
| assignee: | nobody → Dan Streetman (ddstreet) |
| Changed in ebtables (Ubuntu Artful): | |
| assignee: | nobody → Dan Streetman (ddstreet) |
| Changed in ebtables (Ubuntu Xenial): | |
| assignee: | nobody → Dan Streetman (ddstreet) |
| Changed in ebtables (Ubuntu Trusty): | |
| assignee: | nobody → Dan Streetman (ddstreet) |
| Steve Langasek (vorlon) wrote Please test proposed package | #2 |
| Changed in ebtables (Ubuntu Cosmic): | |
| status: | In Progress → Fix Committed |
| Changed in ebtables (Ubuntu Bionic): | |
| status: | In Progress → Fix Committed |
| tags: | added: verification-needed verification-needed-bionic |
| Changed in ebtables (Ubuntu Xenial): | |
| status: | In Progress → Fix Committed |
| tags: | added: verification-needed-xenial |
| Steve Langasek (vorlon) wrote | #3 |
| Changed in ebtables (Ubuntu Artful): | |
| status: | In Progress → Fix Committed |
| tags: | added: verification-needed-artful |
| Steve Langasek (vorlon) wrote | #4 |
| Steve Langasek (vorlon) wrote | #5 |
| Changed in ebtables (Ubuntu Trusty): | |
| status: | In Progress → Fix Committed |
| tags: | added: verification-needed-trusty |
| Launchpad Janitor (janitor) wrote | #6 |
| Changed in ebtables (Ubuntu Cosmic): | |
| status: | Fix Committed → Fix Released |
| Dan Streetman (ddstreet) wrote | #7 |
| Dan Streetman (ddstreet) wrote | #8 |
| Dan Streetman (ddstreet) wrote | #9 |
| Dan Streetman (ddstreet) wrote | #10 |
| Dan Streetman (ddstreet) wrote | #11 |
| tags: |
added: verification-done verification-done-artful verification-done-bionic verification-done-trusty verification-done-xenial removed: verification-needed verification-needed-artful verification-needed-bionic verification-needed-trusty verification-needed-xenial |
| Launchpad Janitor (janitor) wrote | #12 |
| Changed in ebtables (Ubuntu Bionic): | |
| status: | Fix Committed → Fix Released |
| Łukasz Zemczak (sil2100) wrote Update Released | #13 |
| Launchpad Janitor (janitor) wrote | #14 |
| Changed in ebtables (Ubuntu Artful): | |
| status: | Fix Committed → Fix Released |
| Launchpad Janitor (janitor) wrote | #15 |
| Changed in ebtables (Ubuntu Xenial): | |
| status: | Fix Committed → Fix Released |
| Launchpad Janitor (janitor) wrote | #16 |
| Changed in ebtables (Ubuntu Trusty): | |
| status: | Fix Committed → Fix Released |
thanks @setuid for reproducer bash script that I used in description