svn commit: r333699 - in head: lib/libc/sys share/man/man4 sys/netinet/cc
Lawrence Stewart
lstewart at FreeBSD.org
Thu May 17 02:46:29 UTC 2018
Author: lstewart
Date: Thu May 17 02:46:27 2018
New Revision: 333699
URL: https://svnweb.freebsd.org/changeset/base/333699
Log:
Plug a memory leak and potential NULL-pointer dereference introduced in r331214.
Each TCP connection that uses the system default cc_newreno(4) congestion
control algorithm module leaks a "struct newreno" (8 bytes of memory) at
connection initialisation time. The NULL-pointer dereference is only germane
when using the ABE feature, which is disabled by default.
While at it:
- Defer the allocation of memory until it is actually needed given that ABE is
optional and disabled by default.
- Document the ENOMEM errno in getsockopt(2)/setsockopt(2).
- Document ENOMEM and ENOBUFS in tcp(4) as being synonymous given that they are
used interchangeably throughout the code.
- Fix a few other nits also accidentally omitted from the original patch.
Reported by: Harsh Jain on freebsd-net@
Tested by: tjh@
Differential Revision: https://reviews.freebsd.org/D15358
Modified:
head/lib/libc/sys/getsockopt.2
head/share/man/man4/tcp.4
head/sys/netinet/cc/cc_newreno.c
Modified: head/lib/libc/sys/getsockopt.2
==============================================================================
--- head/lib/libc/sys/getsockopt.2 Thu May 17 01:42:18 2018 (r333698)
+++ head/lib/libc/sys/getsockopt.2 Thu May 17 02:46:27 2018 (r333699)
@@ -28,7 +28,7 @@
.\" @(#)getsockopt.2 8.4 (Berkeley) 5/2/95
.\" $FreeBSD$
.\"
-.Dd January 18, 2017
+.Dd May 9, 2018
.Dt GETSOCKOPT 2
.Os
.Sh NAME
@@ -548,6 +548,8 @@ is not in a valid part of the process address space.
Installing an
.Xr accept_filter 9
on a non-listening socket was attempted.
+.It Bq Er ENOMEM
+A memory allocation failed that was required to service the request.
.El
.Sh SEE ALSO
.Xr ioctl 2 ,
Modified: head/share/man/man4/tcp.4
==============================================================================
--- head/share/man/man4/tcp.4 Thu May 17 01:42:18 2018 (r333698)
+++ head/share/man/man4/tcp.4 Thu May 17 02:46:27 2018 (r333699)
@@ -34,7 +34,7 @@
.\" From: @(#)tcp.4 8.1 (Berkeley) 6/5/93
.\" $FreeBSD$
.\"
-.Dd February 6, 2017
+.Dd May 9, 2018
.Dt TCP 4
.Os
.Sh NAME
@@ -599,7 +599,7 @@ A socket operation may fail with one of the following
.It Bq Er EISCONN
when trying to establish a connection on a socket which
already has one;
-.It Bq Er ENOBUFS
+.It Bo Er ENOBUFS Bc or Bo Er ENOMEM Bc
when the system runs out of memory for
an internal data structure;
.It Bq Er ETIMEDOUT
Modified: head/sys/netinet/cc/cc_newreno.c
==============================================================================
--- head/sys/netinet/cc/cc_newreno.c Thu May 17 01:42:18 2018 (r333698)
+++ head/sys/netinet/cc/cc_newreno.c Thu May 17 02:46:27 2018 (r333699)
@@ -81,7 +81,7 @@ static MALLOC_DEFINE(M_NEWRENO, "newreno data",
#define CAST_PTR_INT(X) (*((int*)(X)))
-static int newreno_cb_init(struct cc_var *ccv);
+static void newreno_cb_destroy(struct cc_var *ccv);
static void newreno_ack_received(struct cc_var *ccv, uint16_t type);
static void newreno_after_idle(struct cc_var *ccv);
static void newreno_cong_signal(struct cc_var *ccv, uint32_t type);
@@ -95,7 +95,7 @@ static VNET_DEFINE(uint32_t, newreno_beta_ecn) = 80;
struct cc_algo newreno_cc_algo = {
.name = "newreno",
- .cb_init = newreno_cb_init,
+ .cb_destroy = newreno_cb_destroy,
.ack_received = newreno_ack_received,
.after_idle = newreno_after_idle,
.cong_signal = newreno_cong_signal,
@@ -108,21 +108,31 @@ struct newreno {
uint32_t beta_ecn;
};
-int
-newreno_cb_init(struct cc_var *ccv)
+static inline struct newreno *
+newreno_malloc(struct cc_var *ccv)
{
- struct newreno *nreno;
+ struct newreno *nreno;
- nreno = malloc(sizeof(struct newreno), M_NEWRENO, M_NOWAIT|M_ZERO);
+ nreno = malloc(sizeof(struct newreno), M_NEWRENO, M_NOWAIT);
if (nreno != NULL) {
+ /* NB: nreno is not zeroed, so initialise all fields. */
nreno->beta = V_newreno_beta;
nreno->beta_ecn = V_newreno_beta_ecn;
+ ccv->cc_data = nreno;
}
- return (0);
+ return (nreno);
}
static void
+newreno_cb_destroy(struct cc_var *ccv)
+{
+
+ if (ccv->cc_data != NULL)
+ free(ccv->cc_data, M_NEWRENO);
+}
+
+static void
newreno_ack_received(struct cc_var *ccv, uint16_t type)
{
if (type == CC_ACK && !IN_RECOVERY(CCV(ccv, t_flags)) &&
@@ -224,20 +234,18 @@ static void
newreno_cong_signal(struct cc_var *ccv, uint32_t type)
{
struct newreno *nreno;
- uint32_t cwin, factor;
+ uint32_t beta, beta_ecn, cwin, factor;
u_int mss;
- factor = V_newreno_beta;
- nreno = ccv->cc_data;
- if (nreno != NULL) {
- if (V_cc_do_abe)
- factor = (type == CC_ECN ? nreno->beta_ecn: nreno->beta);
- else
- factor = nreno->beta;
- }
-
cwin = CCV(ccv, snd_cwnd);
mss = CCV(ccv, t_maxseg);
+ nreno = ccv->cc_data;
+ beta = (nreno == NULL) ? V_newreno_beta : nreno->beta;
+ beta_ecn = (nreno == NULL) ? V_newreno_beta_ecn : nreno->beta_ecn;
+ if (V_cc_do_abe && type == CC_ECN)
+ factor = beta_ecn;
+ else
+ factor = beta;
/* Catch algos which mistakenly leak private signal types. */
KASSERT((type & CC_SIGPRIVMASK) == 0,
@@ -253,8 +261,8 @@ newreno_cong_signal(struct cc_var *ccv, uint32_t type)
V_cc_do_abe && V_cc_abe_frlossreduce)) {
CCV(ccv, snd_ssthresh) =
((uint64_t)CCV(ccv, snd_ssthresh) *
- (uint64_t)nreno->beta) /
- (100ULL * (uint64_t)nreno->beta_ecn);
+ (uint64_t)beta) /
+ (100ULL * (uint64_t)beta_ecn);
}
if (!IN_CONGRECOVERY(CCV(ccv, t_flags)))
CCV(ccv, snd_ssthresh) = cwin;
@@ -278,7 +286,6 @@ static void
newreno_post_recovery(struct cc_var *ccv)
{
int pipe;
- pipe = 0;
if (IN_FASTRECOVERY(CCV(ccv, t_flags))) {
/*
@@ -302,7 +309,7 @@ newreno_post_recovery(struct cc_var *ccv)
}
}
-int
+static int
newreno_ctl_output(struct cc_var *ccv, struct sockopt *sopt, void *buf)
{
struct newreno *nreno;
@@ -313,9 +320,15 @@ newreno_ctl_output(struct cc_var *ccv, struct sockopt
nreno = ccv->cc_data;
opt = buf;
-
+
switch (sopt->sopt_dir) {
case SOPT_SET:
+ /* We cannot set without cc_data memory. */
+ if (nreno == NULL) {
+ nreno = newreno_malloc(ccv);
+ if (nreno == NULL)
+ return (ENOMEM);
+ }
switch (opt->name) {
case CC_NEWRENO_BETA:
nreno->beta = opt->val;
@@ -328,17 +341,21 @@ newreno_ctl_output(struct cc_var *ccv, struct sockopt
default:
return (ENOPROTOOPT);
}
+ break;
case SOPT_GET:
switch (opt->name) {
case CC_NEWRENO_BETA:
- opt->val = nreno->beta;
+ opt->val = (nreno == NULL) ?
+ V_newreno_beta : nreno->beta;
break;
case CC_NEWRENO_BETA_ECN:
- opt->val = nreno->beta_ecn;
+ opt->val = (nreno == NULL) ?
+ V_newreno_beta_ecn : nreno->beta_ecn;
break;
default:
return (ENOPROTOOPT);
}
+ break;
default:
return (EINVAL);
}
@@ -349,6 +366,7 @@ newreno_ctl_output(struct cc_var *ccv, struct sockopt
static int
newreno_beta_handler(SYSCTL_HANDLER_ARGS)
{
+
if (req->newptr != NULL ) {
if (arg1 == &VNET_NAME(newreno_beta_ecn) && !V_cc_do_abe)
return (EACCES);
More information about the svn-src-head
mailing list