Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 25 Apr 2024 04:56:20 GMT
From:      Gleb Smirnoff <glebius@FreeBSD.org>
To:        src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org
Subject:   git: 19307b86d31b - main - accept_filter: return different errors for non-listener and a busy socket
Message-ID:  <202404250456.43P4uKKd047422@gitrepo.freebsd.org>

next in thread | raw e-mail | index | archive | help
The branch main has been updated by glebius:

URL: https://cgit.FreeBSD.org/src/commit/?id=19307b86d31b7dea4bb5c3f7e233ee0e59049258

commit 19307b86d31b7dea4bb5c3f7e233ee0e59049258
Author:     Gleb Smirnoff <glebius@FreeBSD.org>
AuthorDate: 2024-04-25 04:50:58 +0000
Commit:     Gleb Smirnoff <glebius@FreeBSD.org>
CommitDate: 2024-04-25 04:55:58 +0000

    accept_filter: return different errors for non-listener and a busy socket
    
    The fact that an accept filter needs to be cleared first before setting to
    a different one isn't properly documented.  The requirement that the
    socket needs already be listening, although trivial, isn't documented
    either.  At least return a more meaningful error than EINVAL for an
    existing filter.  Cover this with a test case.
---
 sys/kern/uipc_accf.c         |  6 +++++-
 tests/sys/kern/socket_accf.c | 32 ++++++++++++++++++++++++++++++++
 2 files changed, 37 insertions(+), 1 deletion(-)

diff --git a/sys/kern/uipc_accf.c b/sys/kern/uipc_accf.c
index c63b5a1179bc..9e30e7839103 100644
--- a/sys/kern/uipc_accf.c
+++ b/sys/kern/uipc_accf.c
@@ -276,10 +276,14 @@ accept_filt_setopt(struct socket *so, struct sockopt *sopt)
 	 * without first removing it.
 	 */
 	SOCK_LOCK(so);
-	if (!SOLISTENING(so) || so->sol_accept_filter != NULL) {
+	if (__predict_false(!SOLISTENING(so))) {
 		error = EINVAL;
 		goto out;
 	}
+	if (__predict_false(so->sol_accept_filter != NULL)) {
+		error = EBUSY;
+		goto out;
+	}
 
 	/*
 	 * Invoke the accf_create() method of the filter if required.  The
diff --git a/tests/sys/kern/socket_accf.c b/tests/sys/kern/socket_accf.c
index 747bcda87010..ae6522397cf7 100644
--- a/tests/sys/kern/socket_accf.c
+++ b/tests/sys/kern/socket_accf.c
@@ -209,11 +209,43 @@ ATF_TC_BODY(tls, tc)
 	ATF_REQUIRE((a = accept(l, NULL, 0)) > 0);
 }
 
+/* Check changing to a different filter. */
+ATF_TC_WITHOUT_HEAD(change);
+ATF_TC_BODY(change, tc)
+{
+	struct accept_filter_arg dfa = {
+		.af_name = "dataready"
+	};
+	struct accept_filter_arg hfa = {
+		.af_name = "httpready"
+	};
+	struct sockaddr_in sin;
+	int n, l;
+
+	l = listensock(&sin);
+	accfon(l, &dfa);
+
+	/* Refuse to change filter without explicit removal of the old one. */
+	ATF_REQUIRE(setsockopt(l, SOL_SOCKET, SO_ACCEPTFILTER, &hfa,
+	    sizeof(hfa)) != 0 && errno == EBUSY);
+
+	/* But allow after clearing. */
+	ATF_REQUIRE(setsockopt(l, SOL_SOCKET, SO_ACCEPTFILTER, NULL, 0) == 0);
+	ATF_REQUIRE(setsockopt(l, SOL_SOCKET, SO_ACCEPTFILTER, &hfa,
+	    sizeof(hfa)) == 0);
+
+	/* Must be listening socket. */
+	ATF_REQUIRE((n = socket(PF_INET, SOCK_STREAM, 0)) > 0);
+	ATF_REQUIRE(setsockopt(n, SOL_SOCKET, SO_ACCEPTFILTER, &dfa,
+	    sizeof(dfa)) != 0 && errno == EINVAL);
+}
+
 ATF_TP_ADD_TCS(tp)
 {
 	ATF_TP_ADD_TC(tp, data);
 	ATF_TP_ADD_TC(tp, http);
 	ATF_TP_ADD_TC(tp, tls);
+	ATF_TP_ADD_TC(tp, change);
 
 	return (atf_no_error());
 }



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202404250456.43P4uKKd047422>