From 89ee337a1fb3d9154ffdb58fcaa098278fd46336 Mon Sep 17 00:00:00 2001 From: Nir Soffer Date: Thu, 14 Jul 2022 00:21:53 +0300 Subject: [PATCH 1/2] gh-94821: Fix autobind of empty unix domain address When binding a unix socket to an empty address on Linux, the socket is automatically bound to an available address in the abstract namespace. >>> s = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM) >>> s.bind("") >>> s.getsockname() b'\x0075499' Since python 3.9, the socket is bound to the one address: >>> s.getsockname() b'\x00' And trying to bind multiple sockets will fail with: Traceback (most recent call last): File "/home/nsoffer/src/cpython/Lib/test/test_socket.py", line 5553, in testAutobind s2.bind("") OSError: [Errno 98] Address already in use Added 2 tests: - Auto binding empty address on Linux - Failing to bind an empty address on other platforms Fixes f6b3a07b7df6 (bpo-44493: Add missing terminated NUL in sockaddr_un's length (GH-26866) --- Lib/test/test_socket.py | 16 ++++++++++++++++ ...2022-07-14-00-43-52.gh-issue-94821.e17ghU.rst | 2 ++ Modules/socketmodule.c | 6 ++++-- 3 files changed, 22 insertions(+), 2 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2022-07-14-00-43-52.gh-issue-94821.e17ghU.rst diff --git a/Lib/test/test_socket.py b/Lib/test/test_socket.py index 1700b429ab07ca..f8617339f5a9ac 100644 --- a/Lib/test/test_socket.py +++ b/Lib/test/test_socket.py @@ -5543,6 +5543,17 @@ def testBytearrayName(self): s.bind(bytearray(b"\x00python\x00test\x00")) self.assertEqual(s.getsockname(), b"\x00python\x00test\x00") + def testAutobind(self): + # Check that binding to an empty string binds to an available address + # in the abstract namespace. + with socket.socket(socket.AF_UNIX, socket.SOCK_STREAM) as s1: + s1.bind("") + self.assertTrue(s1.getsockname().startswith(b"\x00")) + with socket.socket(socket.AF_UNIX, socket.SOCK_STREAM) as s2: + s2.bind("") + self.assertTrue(s2.getsockname().startswith(b"\x00")) + + @unittest.skipUnless(hasattr(socket, 'AF_UNIX'), 'test needs socket.AF_UNIX') class TestUnixDomain(unittest.TestCase): @@ -5612,6 +5623,11 @@ def testUnencodableAddr(self): self.addCleanup(os_helper.unlink, path) self.assertEqual(self.sock.getsockname(), path) + @unittest.skipIf(sys.platform == 'linux', 'Linux specific test') + def testEmptyAddress(self): + # Test that binding empty address fails. + self.assertRaises(OSError, self.sock.bind, "") + class BufferIOTest(SocketConnectedTest): """ diff --git a/Misc/NEWS.d/next/Library/2022-07-14-00-43-52.gh-issue-94821.e17ghU.rst b/Misc/NEWS.d/next/Library/2022-07-14-00-43-52.gh-issue-94821.e17ghU.rst new file mode 100644 index 00000000000000..bf7885aef8cbf9 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2022-07-14-00-43-52.gh-issue-94821.e17ghU.rst @@ -0,0 +1,2 @@ +Fix binding of unix socket to empty address on Linux to use an available +address from the abstract namespace, instead of "\0". diff --git a/Modules/socketmodule.c b/Modules/socketmodule.c index 55e19ffab5cba4..7c19a6acba26e0 100644 --- a/Modules/socketmodule.c +++ b/Modules/socketmodule.c @@ -1724,8 +1724,10 @@ getsockaddrarg(PySocketSockObject *s, PyObject *args, struct sockaddr_un* addr = &addrbuf->un; #ifdef __linux__ - if (path.len > 0 && *(const char *)path.buf == 0) { - /* Linux abstract namespace extension */ + if (path.len == 0 || (path.len > 0 && *(const char *)path.buf == 0)) { + /* Linux abstract namespace extension: + - Empty address auto-binding to an abstract address + - Address that starts with null byte */ if ((size_t)path.len > sizeof addr->sun_path) { PyErr_SetString(PyExc_OSError, "AF_UNIX path too long"); From 2a8bcd73cd48535e3050c18fce1e3e70df104960 Mon Sep 17 00:00:00 2001 From: Nir Soffer Date: Thu, 14 Jul 2022 21:06:18 +0300 Subject: [PATCH 2/2] Address Serhiy comments - Simplify condition since path.len always >= 0 - Make it more clear why we bind to "" twice - Match abstract address pattern - Test that sockets are bound to different addresses --- Lib/test/test_socket.py | 9 ++++++--- Modules/socketmodule.c | 2 +- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/Lib/test/test_socket.py b/Lib/test/test_socket.py index f8617339f5a9ac..8424693fc609a5 100644 --- a/Lib/test/test_socket.py +++ b/Lib/test/test_socket.py @@ -5545,13 +5545,16 @@ def testBytearrayName(self): def testAutobind(self): # Check that binding to an empty string binds to an available address - # in the abstract namespace. + # in the abstract namespace as specified in unix(7) "Autobind feature". + abstract_address = b"^\0[0-9a-f]{5}" with socket.socket(socket.AF_UNIX, socket.SOCK_STREAM) as s1: s1.bind("") - self.assertTrue(s1.getsockname().startswith(b"\x00")) + self.assertRegex(s1.getsockname(), abstract_address) + # Each socket is bound to a different abstract address. with socket.socket(socket.AF_UNIX, socket.SOCK_STREAM) as s2: s2.bind("") - self.assertTrue(s2.getsockname().startswith(b"\x00")) + self.assertRegex(s2.getsockname(), abstract_address) + self.assertNotEqual(s1.getsockname(), s2.getsockname()) @unittest.skipUnless(hasattr(socket, 'AF_UNIX'), 'test needs socket.AF_UNIX') diff --git a/Modules/socketmodule.c b/Modules/socketmodule.c index 7c19a6acba26e0..9b4155e164f107 100644 --- a/Modules/socketmodule.c +++ b/Modules/socketmodule.c @@ -1724,7 +1724,7 @@ getsockaddrarg(PySocketSockObject *s, PyObject *args, struct sockaddr_un* addr = &addrbuf->un; #ifdef __linux__ - if (path.len == 0 || (path.len > 0 && *(const char *)path.buf == 0)) { + if (path.len == 0 || *(const char *)path.buf == 0) { /* Linux abstract namespace extension: - Empty address auto-binding to an abstract address - Address that starts with null byte */