Date: Sat, 10 Jul 2021 10:52:48 +0200 From: Stefan Esser <se@freebsd.org> To: Karl Denninger <karl@denninger.net>, stable@freebsd.org Subject: [PATCH] Re: 12.2 Splay Tree ipfw potential panic source Message-ID: <a06435bb-65c4-c645-031a-dc1bbf121b20@freebsd.org> In-Reply-To: <dde6a01e-c41f-19be-593c-246eef11ea3b@freebsd.org> References: <2e3dcd4d-c8e6-8381-0010-d0844c99901e@denninger.net> <20210708221134.GA32658@belenus.iks-jena.de> <a6a9c220-fee6-a0ea-7721-f88ff865a6a8@denninger.net> <CAFMmRNy9K-1mTDoqQhgdChWV5f_n4QhNesz%2B6xWywn_TQ43xng@mail.gmail.com> <ca5beb7c-db38-1d3c-0f3c-b1b6a12c311e@denninger.net> <7bfda38b-cf81-d8be-7691-e18946e6b56e@denninger.net> <dde6a01e-c41f-19be-593c-246eef11ea3b@freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
This is an OpenPGP/MIME signed message (RFC 4880 and 3156)
--Zgi5MOsookfrqFKU9tjh27pZMUOG0nWRY
Content-Type: multipart/mixed; boundary="yR5sSIA1NjEBz2dgaD9KVGgCH8obpM7Rd";
protected-headers="v1"
From: Stefan Esser <se@freebsd.org>
To: Karl Denninger <karl@denninger.net>, stable@freebsd.org
Message-ID: <a06435bb-65c4-c645-031a-dc1bbf121b20@freebsd.org>
Subject: [PATCH] Re: 12.2 Splay Tree ipfw potential panic source
References: <2e3dcd4d-c8e6-8381-0010-d0844c99901e@denninger.net>
<20210708221134.GA32658@belenus.iks-jena.de>
<a6a9c220-fee6-a0ea-7721-f88ff865a6a8@denninger.net>
<CAFMmRNy9K-1mTDoqQhgdChWV5f_n4QhNesz+6xWywn_TQ43xng@mail.gmail.com>
<ca5beb7c-db38-1d3c-0f3c-b1b6a12c311e@denninger.net>
<7bfda38b-cf81-d8be-7691-e18946e6b56e@denninger.net>
<dde6a01e-c41f-19be-593c-246eef11ea3b@freebsd.org>
In-Reply-To: <dde6a01e-c41f-19be-593c-246eef11ea3b@freebsd.org>
--yR5sSIA1NjEBz2dgaD9KVGgCH8obpM7Rd
Content-Type: multipart/mixed;
boundary="------------C0B4A3C089B174697D5ADE38"
Content-Language: en-US
This is a multi-part message in MIME format.
--------------C0B4A3C089B174697D5ADE38
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: quoted-printable
Am 10.07.21 um 10:23 schrieb Stefan Esser:
> Am 10.07.21 um 04:41 schrieb Karl Denninger:
>> Ok, so I have good news and bad news.
>>
>> I have the trap and it is definitely in libalias which appears to come=
about as
>> a result of a NAT translation attempt.
>>
>> Fatal trap 18: integer divide fault while in kernel mode
> [...]
>> HouseKeeping() at HouseKeeping+0x1c/frame 0xfffffe0017b6b320
>=20
> The divide by zero at one of the first instructions of HouseKeeping()
> seems to be caused by this line:
>=20
> /sys/netinet/libalias/alias_db.c:1753:
>=20
> if (packets % packet_limit =3D=3D 0) {
>=20
> Seems that packet_limit can become zero, there ...
>=20
> At line 1780 within that function:
>=20
> if (now !=3D LibAliasTime) {
> /* retry three times a second */
> packet_limit =3D packets / 3;
> packets =3D 0;
> LibAliasTime =3D now;
> }
>=20
> The static variable packet limit is divided by 3 without any
> protection against going down to 0.
>=20
> A packet_limit of zero makes no sense (besides causing a divide
> by zero abort), therefore this value should probably have a lower
> limit of 1.
>=20
> Maybe that
> packet_limit =3D packets / 3 + 1;
>=20
> would give an acceptably close result in all cases.
>=20
> Else enforce a minimum value of 1 after the division:
>=20
> packet_limit =3D packets / 3;
> if (packet_limit =3D=3D 0)
> packet_limit =3D 1;
> Or just:
> packet_limit =3D packets >=3D 3 ? packets / 3 :=
1;
>=20
> Regards, STefan
I have just noticed that enforcing a lower limit of 1 is totally
equivalent to testing for zero before performing the modulo operation.
The attached patch should fix the panic and does not change the way
packet_limit is calculated. Since the variable is immediately used
in the modulo when not zero, the additional cost of the test for zero
is extremely low, less than that of the other suggested changes.
Maybe that increasing packet_limit by 1 is sensible, anyway, since at
low packet rates it will result in 0 to 5 packets giving the same
effect (the condition in line 1753 evaluates to true).
Anyway, please try the attached patch, which will fix the divide by
zero panic.
Regards, STefan
PS: Patch inline in case it is stripped by the mail-list:
diff --git a/sys/netinet/libalias/alias_db.c b/sys/netinet/libalias/alias=
_db.c
index c09ad4352ce4..d5dec0709cbe 100644
--- a/sys/netinet/libalias/alias_db.c
+++ b/sys/netinet/libalias/alias_db.c
@@ -1769,7 +1769,7 @@ HouseKeeping(struct libalias *la)
* Reduce the amount of house keeping work substantially by
* sampling over the packets.
*/
- if (packets % packet_limit =3D=3D 0) {
+ if (packet_limit =3D=3D 0 || packets % packet_limit =3D=3D 0) {
time_t now;
#ifdef _KERNEL
(Line numbers from -CURRENT, may be slightly off for stable/12.)
--------------C0B4A3C089B174697D5ADE38
Content-Type: text/plain; charset=UTF-8; x-mac-type="0"; x-mac-creator="0";
name="libalias-alias_db.diff"
Content-Transfer-Encoding: base64
Content-Disposition: attachment;
filename="libalias-alias_db.diff"
ZGlmZiAtLWdpdCBhL3N5cy9uZXRpbmV0L2xpYmFsaWFzL2FsaWFzX2RiLmMgYi9zeXMvbmV0
aW5ldC9saWJhbGlhcy9hbGlhc19kYi5jCmluZGV4IGMwOWFkNDM1MmNlNC4uZDVkZWMwNzA5
Y2JlIDEwMDY0NAotLS0gYS9zeXMvbmV0aW5ldC9saWJhbGlhcy9hbGlhc19kYi5jCisrKyBi
L3N5cy9uZXRpbmV0L2xpYmFsaWFzL2FsaWFzX2RiLmMKQEAgLTE3NjksNyArMTc2OSw3IEBA
IEhvdXNlS2VlcGluZyhzdHJ1Y3QgbGliYWxpYXMgKmxhKQogCSAqIFJlZHVjZSB0aGUgYW1v
dW50IG9mIGhvdXNlIGtlZXBpbmcgd29yayBzdWJzdGFudGlhbGx5IGJ5CiAJICogc2FtcGxp
bmcgb3ZlciB0aGUgcGFja2V0cy4KIAkgKi8KLQlpZiAocGFja2V0cyAlIHBhY2tldF9saW1p
dCA9PSAwKSB7CisJaWYgKHBhY2tldF9saW1pdCA9PSAwIHx8IHBhY2tldHMgJSBwYWNrZXRf
bGltaXQgPT0gMCkgewogCQl0aW1lX3Qgbm93OwogCiAjaWZkZWYgX0tFUk5FTAo=
--------------C0B4A3C089B174697D5ADE38--
--yR5sSIA1NjEBz2dgaD9KVGgCH8obpM7Rd--
--Zgi5MOsookfrqFKU9tjh27pZMUOG0nWRY
Content-Type: application/pgp-signature; name="OpenPGP_signature.asc"
Content-Description: OpenPGP digital signature
Content-Disposition: attachment; filename="OpenPGP_signature"
-----BEGIN PGP SIGNATURE-----
wsB5BAABCAAjFiEEo3HqZZwL7MgrcVMTR+u171r99UQFAmDpX+AFAwAAAAAACgkQR+u171r99URf
RggAsGOZTBy3K7+vSzQDZnAVdvPbw3XM8Cv1xtVLMRna3/J8WZ03FR3c+AYCDebrrcDPGo8P0niQ
ETQjoHKL+gz3cTrFkDAh29oWarzRiB9Bxi1Dw+maLE5Cagv6JBtvyNjvTV5us3jStxIAOI8RzmYw
3mC4oUJQKUFlvTwpwSc2O/oyW2z21kzznNgAwmvNJjuhRX9ZQeOZfYm/hd/Zp3gpbHGpDcP5+uHn
cpU5qQjud5rBNSCdzwxifuKhsD0QMU3P6x9yGZg4aCMznZHoQKAtg8V/R6eXrzuNERl5mNmkVaph
jlGEUdwMcVtX/DQ1ap9WJY/Hb3ZGtVWXS0L9ctG6wA==
=Xxoj
-----END PGP SIGNATURE-----
--Zgi5MOsookfrqFKU9tjh27pZMUOG0nWRY--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?a06435bb-65c4-c645-031a-dc1bbf121b20>