Skip to content

Commit e4aa446

Browse files
mtth-bfftl0kod
authored andcommitted
selftests/landlock: NULL-terminate unix pathname addresses
The size of Unix pathname addresses is computed in selftests using offsetof(struct sockaddr_un, sun_path) + strlen(xxx). It should have been that +1, which makes addresses passed to the libc and kernel non-NULL-terminated. unix_mkname_bsd() fixes that in Linux so there is no harm, but just using sizeof(the address struct) should improve readability. Signed-off-by: Matthieu Buffet <matthieu@buffet.re> Reviewed-by: Günther Noack <gnoack@google.com> Link: https://lore.kernel.org/r/20251202215141.689986-1-matthieu@buffet.re Signed-off-by: Mickaël Salaün <mic@digikod.net>
1 parent e1a57c3 commit e4aa446

2 files changed

Lines changed: 20 additions & 25 deletions

File tree

tools/testing/selftests/landlock/fs_test.c

Lines changed: 11 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -4362,22 +4362,24 @@ TEST_F_FORK(layout1, named_unix_domain_socket_ioctl)
43624362
{
43634363
const char *const path = file1_s1d1;
43644364
int srv_fd, cli_fd, ruleset_fd;
4365-
socklen_t size;
4366-
struct sockaddr_un srv_un, cli_un;
4365+
struct sockaddr_un srv_un = {
4366+
.sun_family = AF_UNIX,
4367+
};
4368+
struct sockaddr_un cli_un = {
4369+
.sun_family = AF_UNIX,
4370+
};
43674371
const struct landlock_ruleset_attr attr = {
43684372
.handled_access_fs = LANDLOCK_ACCESS_FS_IOCTL_DEV,
43694373
};
43704374

43714375
/* Sets up a server */
4372-
srv_un.sun_family = AF_UNIX;
4373-
strncpy(srv_un.sun_path, path, sizeof(srv_un.sun_path));
4374-
43754376
ASSERT_EQ(0, unlink(path));
43764377
srv_fd = socket(AF_UNIX, SOCK_STREAM, 0);
43774378
ASSERT_LE(0, srv_fd);
43784379

4379-
size = offsetof(struct sockaddr_un, sun_path) + strlen(srv_un.sun_path);
4380-
ASSERT_EQ(0, bind(srv_fd, (struct sockaddr *)&srv_un, size));
4380+
strncpy(srv_un.sun_path, path, sizeof(srv_un.sun_path));
4381+
ASSERT_EQ(0, bind(srv_fd, (struct sockaddr *)&srv_un, sizeof(srv_un)));
4382+
43814383
ASSERT_EQ(0, listen(srv_fd, 10 /* qlen */));
43824384

43834385
/* Enables Landlock. */
@@ -4387,16 +4389,12 @@ TEST_F_FORK(layout1, named_unix_domain_socket_ioctl)
43874389
ASSERT_EQ(0, close(ruleset_fd));
43884390

43894391
/* Sets up a client connection to it */
4390-
cli_un.sun_family = AF_UNIX;
43914392
cli_fd = socket(AF_UNIX, SOCK_STREAM, 0);
43924393
ASSERT_LE(0, cli_fd);
43934394

4394-
bzero(&cli_un, sizeof(cli_un));
4395-
cli_un.sun_family = AF_UNIX;
43964395
strncpy(cli_un.sun_path, path, sizeof(cli_un.sun_path));
4397-
size = offsetof(struct sockaddr_un, sun_path) + strlen(cli_un.sun_path);
4398-
4399-
ASSERT_EQ(0, connect(cli_fd, (struct sockaddr *)&cli_un, size));
4396+
ASSERT_EQ(0,
4397+
connect(cli_fd, (struct sockaddr *)&cli_un, sizeof(cli_un)));
44004398

44014399
/* FIONREAD and other IOCTLs should not be forbidden. */
44024400
EXPECT_EQ(0, test_fionread_ioctl(cli_fd));

tools/testing/selftests/landlock/scoped_abstract_unix_test.c

Lines changed: 9 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -779,7 +779,6 @@ FIXTURE_TEARDOWN(various_address_sockets)
779779

780780
TEST_F(various_address_sockets, scoped_pathname_sockets)
781781
{
782-
socklen_t size_stream, size_dgram;
783782
pid_t child;
784783
int status;
785784
char buf_child, buf_parent;
@@ -798,12 +797,8 @@ TEST_F(various_address_sockets, scoped_pathname_sockets)
798797
/* Pathname address. */
799798
snprintf(stream_pathname_addr.sun_path,
800799
sizeof(stream_pathname_addr.sun_path), "%s", stream_path);
801-
size_stream = offsetof(struct sockaddr_un, sun_path) +
802-
strlen(stream_pathname_addr.sun_path);
803800
snprintf(dgram_pathname_addr.sun_path,
804801
sizeof(dgram_pathname_addr.sun_path), "%s", dgram_path);
805-
size_dgram = offsetof(struct sockaddr_un, sun_path) +
806-
strlen(dgram_pathname_addr.sun_path);
807802

808803
/* Abstract address. */
809804
memset(&stream_abstract_addr, 0, sizeof(stream_abstract_addr));
@@ -841,21 +836,23 @@ TEST_F(various_address_sockets, scoped_pathname_sockets)
841836
/* Connects with pathname sockets. */
842837
stream_pathname_socket = socket(AF_UNIX, SOCK_STREAM, 0);
843838
ASSERT_LE(0, stream_pathname_socket);
844-
ASSERT_EQ(0, connect(stream_pathname_socket,
845-
&stream_pathname_addr, size_stream));
839+
ASSERT_EQ(0,
840+
connect(stream_pathname_socket, &stream_pathname_addr,
841+
sizeof(stream_pathname_addr)));
846842
ASSERT_EQ(1, write(stream_pathname_socket, "b", 1));
847843
EXPECT_EQ(0, close(stream_pathname_socket));
848844

849845
/* Sends without connection. */
850846
dgram_pathname_socket = socket(AF_UNIX, SOCK_DGRAM, 0);
851847
ASSERT_LE(0, dgram_pathname_socket);
852848
err = sendto(dgram_pathname_socket, "c", 1, 0,
853-
&dgram_pathname_addr, size_dgram);
849+
&dgram_pathname_addr, sizeof(dgram_pathname_addr));
854850
EXPECT_EQ(1, err);
855851

856852
/* Sends with connection. */
857-
ASSERT_EQ(0, connect(dgram_pathname_socket,
858-
&dgram_pathname_addr, size_dgram));
853+
ASSERT_EQ(0,
854+
connect(dgram_pathname_socket, &dgram_pathname_addr,
855+
sizeof(dgram_pathname_addr)));
859856
ASSERT_EQ(1, write(dgram_pathname_socket, "d", 1));
860857
EXPECT_EQ(0, close(dgram_pathname_socket));
861858

@@ -910,13 +907,13 @@ TEST_F(various_address_sockets, scoped_pathname_sockets)
910907
stream_pathname_socket = socket(AF_UNIX, SOCK_STREAM, 0);
911908
ASSERT_LE(0, stream_pathname_socket);
912909
ASSERT_EQ(0, bind(stream_pathname_socket, &stream_pathname_addr,
913-
size_stream));
910+
sizeof(stream_pathname_addr)));
914911
ASSERT_EQ(0, listen(stream_pathname_socket, backlog));
915912

916913
dgram_pathname_socket = socket(AF_UNIX, SOCK_DGRAM, 0);
917914
ASSERT_LE(0, dgram_pathname_socket);
918915
ASSERT_EQ(0, bind(dgram_pathname_socket, &dgram_pathname_addr,
919-
size_dgram));
916+
sizeof(dgram_pathname_addr)));
920917

921918
/* Sets up abstract servers. */
922919
stream_abstract_socket = socket(AF_UNIX, SOCK_STREAM, 0);

0 commit comments

Comments
 (0)