From 505a9bce029e51540739c853a6c9eef0ecfb2e90 Mon Sep 17 00:00:00 2001 From: Garrett D'Amore Date: Wed, 4 Apr 2018 11:07:56 -0700 Subject: fixes #329 type checking not done for setopt --- src/core/endpt.c | 6 +- src/core/options.c | 197 ++++++++++++++++++++++++++++++--------------------- src/core/options.h | 38 ++++------ src/core/protocol.h | 2 +- src/core/socket.c | 98 +++++++++++++++---------- src/core/transport.c | 4 +- src/core/transport.h | 4 +- 7 files changed, 197 insertions(+), 152 deletions(-) (limited to 'src/core') diff --git a/src/core/endpt.c b/src/core/endpt.c index d4bc9b01..0cb59a14 100644 --- a/src/core/endpt.c +++ b/src/core/endpt.c @@ -598,13 +598,9 @@ nni_ep_setopt(nni_ep *ep, const char *name, const void *val, size_t sz, int t) if (eo->eo_setopt == NULL) { return (NNG_EREADONLY); } - if ((t != NNI_TYPE_OPAQUE) && - (eo->eo_type != NNI_TYPE_OPAQUE) && (t != eo->eo_type)) { - return (NNG_EBADTYPE); - } nni_mtx_lock(&ep->ep_mtx); - rv = eo->eo_setopt(ep->ep_data, val, sz); + rv = eo->eo_setopt(ep->ep_data, val, sz, t); nni_mtx_unlock(&ep->ep_mtx); return (rv); } diff --git a/src/core/options.c b/src/core/options.c index 9410de5f..e0c72d25 100644 --- a/src/core/options.c +++ b/src/core/options.c @@ -14,139 +14,174 @@ #include int -nni_chkopt_ms(const void *v, size_t sz) +nni_copyin_ms(nni_duration *dp, const void *v, size_t sz, int typ) { - nni_duration val; - if (sz != sizeof(val)) { - return (NNG_EINVAL); - } - memcpy(&val, v, sz); - if (val < -1) { - return (NNG_EINVAL); - } - return (0); -} + nni_duration dur; -int -nni_chkopt_int(const void *v, size_t sz, int minv, int maxv) -{ - int val; - if (sz != sizeof(val)) { - return (NNG_EINVAL); + switch (typ) { + case NNI_TYPE_DURATION: + dur = *(nng_duration *) v; + break; + case NNI_TYPE_OPAQUE: + if (sz != sizeof(dur)) { + return (NNG_EINVAL); + } + memcpy(&dur, v, sz); + break; + default: + return (NNG_EBADTYPE); } - memcpy(&val, v, sz); - if ((val < minv) || (val > maxv)) { + + if (dur < -1) { return (NNG_EINVAL); } + *dp = dur; return (0); } int -nni_chkopt_bool(size_t sz) +nni_copyin_bool(bool *bp, const void *v, size_t sz, int typ) { - if (sz != sizeof(bool)) { - return (NNG_EINVAL); + switch (typ) { + case NNI_TYPE_BOOL: + *bp = *(bool *) v; + break; + case NNI_TYPE_OPAQUE: + if (sz != sizeof(bool)) { + return (NNG_EINVAL); + } + memcpy(bp, v, sz); + break; + default: + return (NNG_EBADTYPE); } + return (0); } int -nni_chkopt_size(const void *v, size_t sz, size_t minv, size_t maxv) +nni_copyin_int(int *ip, const void *v, size_t sz, int minv, int maxv, int typ) { - size_t val; - if (sz != sizeof(val)) { + int i; + + switch (typ) { + case NNI_TYPE_INT32: + i = *(int *) v; + break; + case NNI_TYPE_OPAQUE: + if (sz != sizeof(i)) { + return (NNG_EINVAL); + } + memcpy(&i, v, sz); + break; + default: + return (NNG_EBADTYPE); + } + if (i > maxv) { return (NNG_EINVAL); } - memcpy(&val, v, sz); - if ((val < minv) || (val > maxv)) { + if (i < minv) { return (NNG_EINVAL); } + *ip = i; return (0); } int -nni_setopt_ms(nni_duration *dp, const void *v, size_t sz) +nni_copyin_size( + size_t *sp, const void *v, size_t sz, size_t minv, size_t maxv, int typ) { - nni_duration dur; + size_t val; - if (sz != sizeof(*dp)) { - return (NNG_EINVAL); - } - memcpy(&dur, v, sizeof(dur)); - if (dur < -1) { - return (NNG_EINVAL); + switch (typ) { + case NNI_TYPE_SIZE: + val = *(size_t *) v; + break; + case NNI_TYPE_OPAQUE: + if (sz != sizeof(val)) { + return (NNG_EINVAL); + } + memcpy(&val, v, sz); + break; + default: + return (NNG_EBADTYPE); } - *dp = dur; - return (0); -} -int -nni_setopt_bool(bool *bp, const void *v, size_t sz) -{ - if (sz != sizeof(*bp)) { + val = *(size_t *) v; + if ((val > maxv) || (val < minv)) { return (NNG_EINVAL); } - memcpy(bp, v, sizeof(*bp)); + *sp = val; return (0); } int -nni_setopt_int(int *ip, const void *v, size_t sz, int minv, int maxv) +nni_copyin_ptr(void **pp, const void *v, size_t sz, int typ) { - int i; + void *p; - if (sz != sizeof(i)) { - return (NNG_EINVAL); - } - memcpy(&i, v, sizeof(i)); - if (i > maxv) { - return (NNG_EINVAL); - } - if (i < minv) { - return (NNG_EINVAL); + switch (typ) { + case NNI_TYPE_POINTER: + p = *(void **) v; + break; + case NNI_TYPE_OPAQUE: + if (sz != sizeof(p)) { + return (NNG_EINVAL); + } + memcpy(&p, v, sz); + break; + default: + return (NNG_EBADTYPE); } - *ip = i; + *pp = p; return (0); } int -nni_setopt_size(size_t *sp, const void *v, size_t sz, size_t minv, size_t maxv) +nni_copyin_str(char *s, const void *v, size_t sz, size_t maxsz, int typ) { - size_t val; + size_t z; - if (sz != sizeof(val)) { - return (NNG_EINVAL); - } - memcpy(&val, v, sizeof(val)); - if (val > maxv) { - return (NNG_EINVAL); + switch (typ) { + case NNI_TYPE_STRING: + z = strlen(v) + 1; + NNI_ASSERT(sz == z); + break; + case NNI_TYPE_OPAQUE: + if ((z = nni_strnlen(v, sz)) >= sz) { + return (NNG_EINVAL); // missing terminator + } + break; + default: + return (NNG_EBADTYPE); } - if (val < minv) { - return (NNG_EINVAL); + if (z > maxsz) { + return (NNG_EINVAL); // too long } - *sp = val; + memcpy(s, v, z); return (0); } int -nni_setopt_buf(nni_msgq *mq, const void *val, size_t sz) +nni_copyin_u64(uint64_t *up, const void *v, size_t sz, int typ) { - int len; + uint64_t u; - if (sz < sizeof(len)) { - return (NNG_EINVAL); - } - memcpy(&len, val, sizeof(len)); - if (len < 0) { - return (NNG_EINVAL); - } - if (len > 8192) { - // put a reasonable uppper limit on queue depth. - // This is a count in messages, so the total queue - // size could be quite large indeed in this case. - return (NNG_EINVAL); + switch (typ) { + case NNI_TYPE_UINT64: + u = *(uint64_t *) v; + break; + case NNI_TYPE_OPAQUE: + if (sz != sizeof(u)) { + return (NNG_EINVAL); + } + memcpy(&u, v, sz); + break; + default: + return (NNG_EBADTYPE); } - return (nni_msgq_resize(mq, len)); + *up = u; + return (0); } int diff --git a/src/core/options.h b/src/core/options.h index 35c3232f..dfc15d31 100644 --- a/src/core/options.h +++ b/src/core/options.h @@ -11,39 +11,27 @@ #ifndef CORE_OPTIONS_H #define CORE_OPTIONS_H -// Option helpers. These can be called from protocols or transports -// in their own option handling, centralizing the logic for dealing with -// variable sized options. - -// nni_setopt_buf sets the queue size for the message queue. -extern int nni_setopt_buf(nni_msgq *, const void *, size_t); - -// nni_setopt_duration sets the duration. Durations must be legal, -// either a positive value, 0, or -1 to indicate forever. -extern int nni_setopt_ms(nni_duration *, const void *, size_t); - -// nni_setopt_bool sets a bool, or _Bool -extern int nni_setopt_bool(bool *, const void *, size_t); - -// nni_setopt_int sets an integer, which must be between the minimum and -// maximum values (inclusive). -extern int nni_setopt_int(int *, const void *, size_t, int, int); - +// Integer limits. #define NNI_MAXINT ((int) 2147483647) #define NNI_MININT ((int) -2147483648) -// nni_setopt_size sets a size_t option. -extern int nni_setopt_size(size_t *, const void *, size_t, size_t, size_t); - // We limit the maximum size to 4GB. That's intentional because some of the // underlying protocols cannot cope with anything bigger than 32-bits. #define NNI_MINSZ (0) #define NNI_MAXSZ ((size_t) 0xffffffff) -extern int nni_chkopt_bool(size_t); -extern int nni_chkopt_ms(const void *, size_t); -extern int nni_chkopt_int(const void *, size_t, int, int); -extern int nni_chkopt_size(const void *, size_t, size_t, size_t); +// Option helpers. These can be called from protocols or transports +// in their own option handling, centralizing the logic for dealing with +// variable sized options. + +extern int nni_copyin_ms(nni_duration *, const void *, size_t, int); +extern int nni_copyin_bool(bool *, const void *, size_t, int); +extern int nni_copyin_int(int *, const void *, size_t, int, int, int); +extern int nni_copyin_size( + size_t *, const void *, size_t, size_t, size_t, int); +extern int nni_copyin_str(char *, const void *, size_t, size_t, int); +extern int nni_copyin_ptr(void **, const void *, size_t, int); +extern int nni_copyin_u64(uint64_t *, const void *, size_t, int); // nni_copyout_xxx copies out a type of the named value. It assumes that // the type is aligned and the size correct, unless NNI_TYPE_OPAQUE is passed. diff --git a/src/core/protocol.h b/src/core/protocol.h index 1c341241..01e3d11c 100644 --- a/src/core/protocol.h +++ b/src/core/protocol.h @@ -51,7 +51,7 @@ struct nni_proto_sock_option { const char *pso_name; int pso_type; int (*pso_getopt)(void *, void *, size_t *, int); - int (*pso_setopt)(void *, const void *, size_t); + int (*pso_setopt)(void *, const void *, size_t, int); }; struct nni_proto_sock_ops { diff --git a/src/core/socket.c b/src/core/socket.c index 9e98a9d9..40ef32f3 100644 --- a/src/core/socket.c +++ b/src/core/socket.c @@ -23,7 +23,7 @@ typedef struct nni_socket_option { const char *so_name; int so_type; int (*so_getopt)(nni_sock *, void *, size_t *, int); - int (*so_setopt)(nni_sock *, const void *, size_t); + int (*so_setopt)(nni_sock *, const void *, size_t, int); } nni_socket_option; typedef struct nni_sockopt { @@ -99,6 +99,32 @@ nni_sock_can_recv_cb(void *arg, int flags) } } +void +nni_sock_set_sendable(nni_sock *s, bool cansend) +{ + nni_notifyfd *fd = &s->s_send_fd; + if (fd->sn_init) { + if (cansend) { + nni_plat_pipe_raise(fd->sn_wfd); + } else { + nni_plat_pipe_clear(fd->sn_rfd); + } + } +} + +void +nni_sock_set_recvable(nni_sock *s, bool canrecv) +{ + nni_notifyfd *fd = &s->s_recv_fd; + if (fd->sn_init) { + if (canrecv) { + nni_plat_pipe_raise(fd->sn_wfd); + } else { + nni_plat_pipe_clear(fd->sn_rfd); + } + } +} + static int nni_sock_get_fd(nni_sock *s, int flag, int *fdp) { @@ -167,9 +193,9 @@ nni_sock_getopt_recvfd(nni_sock *s, void *buf, size_t *szp, int typ) } static int -nni_sock_setopt_recvtimeo(nni_sock *s, const void *buf, size_t sz) +nni_sock_setopt_recvtimeo(nni_sock *s, const void *buf, size_t sz, int typ) { - return (nni_setopt_ms(&s->s_rcvtimeo, buf, sz)); + return (nni_copyin_ms(&s->s_rcvtimeo, buf, sz, typ)); } static int @@ -179,9 +205,9 @@ nni_sock_getopt_recvtimeo(nni_sock *s, void *buf, size_t *szp, int typ) } static int -nni_sock_setopt_sendtimeo(nni_sock *s, const void *buf, size_t sz) +nni_sock_setopt_sendtimeo(nni_sock *s, const void *buf, size_t sz, int typ) { - return (nni_setopt_ms(&s->s_sndtimeo, buf, sz)); + return (nni_copyin_ms(&s->s_sndtimeo, buf, sz, typ)); } static int @@ -191,9 +217,9 @@ nni_sock_getopt_sendtimeo(nni_sock *s, void *buf, size_t *szp, int typ) } static int -nni_sock_setopt_reconnmint(nni_sock *s, const void *buf, size_t sz) +nni_sock_setopt_reconnmint(nni_sock *s, const void *buf, size_t sz, int typ) { - return (nni_setopt_ms(&s->s_reconn, buf, sz)); + return (nni_copyin_ms(&s->s_reconn, buf, sz, typ)); } static int @@ -203,9 +229,9 @@ nni_sock_getopt_reconnmint(nni_sock *s, void *buf, size_t *szp, int typ) } static int -nni_sock_setopt_reconnmaxt(nni_sock *s, const void *buf, size_t sz) +nni_sock_setopt_reconnmaxt(nni_sock *s, const void *buf, size_t sz, int typ) { - return (nni_setopt_ms(&s->s_reconnmax, buf, sz)); + return (nni_copyin_ms(&s->s_reconnmax, buf, sz, typ)); } static int @@ -215,9 +241,15 @@ nni_sock_getopt_reconnmaxt(nni_sock *s, void *buf, size_t *szp, int typ) } static int -nni_sock_setopt_recvbuf(nni_sock *s, const void *buf, size_t sz) +nni_sock_setopt_recvbuf(nni_sock *s, const void *buf, size_t sz, int typ) { - return (nni_setopt_buf(s->s_urq, buf, sz)); + int len; + int rv; + + if ((rv = nni_copyin_int(&len, buf, sz, 0, 8192, typ)) != 0) { + return (rv); + } + return (nni_msgq_resize(s->s_urq, len)); } static int @@ -229,9 +261,15 @@ nni_sock_getopt_recvbuf(nni_sock *s, void *buf, size_t *szp, int typ) } static int -nni_sock_setopt_sendbuf(nni_sock *s, const void *buf, size_t sz) +nni_sock_setopt_sendbuf(nni_sock *s, const void *buf, size_t sz, int typ) { - return (nni_setopt_buf(s->s_uwq, buf, sz)); + int len; + int rv; + + if ((rv = nni_copyin_int(&len, buf, sz, 0, 8192, typ)) != 0) { + return (rv); + } + return (nni_msgq_resize(s->s_uwq, len)); } static int @@ -249,13 +287,9 @@ nni_sock_getopt_sockname(nni_sock *s, void *buf, size_t *szp, int typ) } static int -nni_sock_setopt_sockname(nni_sock *s, const void *buf, size_t sz) +nni_sock_setopt_sockname(nni_sock *s, const void *buf, size_t sz, int typ) { - if (nni_strnlen(buf, sz) > sizeof(s->s_name) - 1) { - return (NNG_EINVAL); - } - nni_strlcpy(s->s_name, buf, sizeof(s->s_name)); - return (0); + return (nni_copyin_str(s->s_name, buf, sizeof(s->s_name), sz, typ)); } static const nni_socket_option nni_sock_options[] = { @@ -748,7 +782,7 @@ nni_sock_close(nni_sock *s) } nni_mtx_unlock(&nni_sock_lk); - // Wait for pipe and eps to finish closing. + // Wait for pipes, eps, and contexts to finish closing. nni_mtx_lock(&s->s_mx); while ( (!nni_list_empty(&s->s_pipes)) || (!nni_list_empty(&s->s_eps))) { @@ -895,12 +929,7 @@ nni_sock_setopt(nni_sock *s, const char *name, const void *v, size_t sz, int t) nni_mtx_unlock(&s->s_mx); return (NNG_EREADONLY); } - if ((pso->pso_type != NNI_TYPE_OPAQUE) && - (t != NNI_TYPE_OPAQUE) && (t != pso->pso_type)) { - nni_mtx_unlock(&s->s_mx); - return (NNG_EBADTYPE); - } - rv = pso->pso_setopt(s->s_data, v, sz); + rv = pso->pso_setopt(s->s_data, v, sz, t); nni_mtx_unlock(&s->s_mx); return (rv); } @@ -914,12 +943,7 @@ nni_sock_setopt(nni_sock *s, const char *name, const void *v, size_t sz, int t) nni_mtx_unlock(&s->s_mx); return (NNG_EREADONLY); } - if ((sso->so_type != NNI_TYPE_OPAQUE) && - (t != NNI_TYPE_OPAQUE) && (t != sso->so_type)) { - nni_mtx_unlock(&s->s_mx); - return (NNG_EBADTYPE); - } - rv = sso->so_setopt(s, v, sz); + rv = sso->so_setopt(s, v, sz, t); nni_mtx_unlock(&s->s_mx); return (rv); } @@ -933,17 +957,19 @@ nni_sock_setopt(nni_sock *s, const char *name, const void *v, size_t sz, int t) // Validation of transport options. This is stateless, so transports // should not fail to set an option later if they passed it here. - rv = nni_tran_chkopt(name, v, sz); + rv = nni_tran_chkopt(name, v, sz, t); // Also check a few generic things. We do this if no transport // was found, or even if a transport rejected one of the settings. if ((rv == NNG_ENOTSUP) || (rv == 0)) { if ((strcmp(name, NNG_OPT_LINGER) == 0)) { - rv = nni_chkopt_ms(v, sz); + nng_duration d; + rv = nni_copyin_ms(&d, v, sz, t); } else if (strcmp(name, NNG_OPT_RECVMAXSZ) == 0) { + size_t z; // just a sanity test on the size; it also ensures that // a size can be set even with no transport configured. - rv = nni_chkopt_size(v, sz, 0, NNI_MAXSZ); + rv = nni_copyin_size(&z, v, sz, 0, NNI_MAXSZ, t); } } @@ -1014,7 +1040,7 @@ nni_sock_setopt(nni_sock *s, const char *name, const void *v, size_t sz, int t) // will already have had a chance to veto this. if (strcmp(name, NNG_OPT_LINGER) == 0) { - rv = nni_setopt_ms(&s->s_linger, v, sz); + rv = nni_copyin_ms(&s->s_linger, v, sz, t); } if (rv == 0) { diff --git a/src/core/transport.c b/src/core/transport.c index b48c7da6..6c872b81 100644 --- a/src/core/transport.c +++ b/src/core/transport.c @@ -98,7 +98,7 @@ nni_tran_find(nni_url *url) } int -nni_tran_chkopt(const char *name, const void *v, size_t sz) +nni_tran_chkopt(const char *name, const void *v, size_t sz, int typ) { nni_transport *t; int rv = NNG_ENOTSUP; @@ -118,7 +118,7 @@ nni_tran_chkopt(const char *name, const void *v, size_t sz) nni_mtx_unlock(&nni_tran_lk); return (NNG_EREADONLY); } - if ((rv = eo->eo_setopt(NULL, v, sz)) != 0) { + if ((rv = eo->eo_setopt(NULL, v, sz, typ)) != 0) { nni_mtx_unlock(&nni_tran_lk); return (rv); } diff --git a/src/core/transport.h b/src/core/transport.h index 7085126f..b1fecaa2 100644 --- a/src/core/transport.h +++ b/src/core/transport.h @@ -65,7 +65,7 @@ struct nni_tran_ep_option { // performed, but the option should be sanity tested for presence // and size. (This permits the core to validate that an option // is reasonable and be set even before endpoints are created.) - int (*eo_setopt)(void *, const void *, size_t); + int (*eo_setopt)(void *, const void *, size_t, int); }; // Endpoint operations are called by the socket in a protocol-independent @@ -172,7 +172,7 @@ struct nni_tran_pipe { // These APIs are used by the framework internally, and not for use by // transport implementations. extern nni_tran *nni_tran_find(nni_url *); -extern int nni_tran_chkopt(const char *, const void *, size_t); +extern int nni_tran_chkopt(const char *, const void *, size_t, int); extern int nni_tran_sys_init(void); extern void nni_tran_sys_fini(void); extern int nni_tran_register(const nni_tran *); -- cgit v1.2.3-70-g09d2