From patchwork Wed Mar 27 17:14:00 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Gregory Rose X-Patchwork-Id: 1067226 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=openvswitch.org (client-ip=140.211.169.12; helo=mail.linuxfoundation.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Authentication-Results: ozlabs.org; dmarc=fail (p=none dis=none) header.from=gmail.com Authentication-Results: ozlabs.org; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.b="o/qQNBfO"; dkim-atps=neutral Received: from mail.linuxfoundation.org (mail.linuxfoundation.org [140.211.169.12]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 44Tvk10sW8z9s70 for ; Thu, 28 Mar 2019 04:14:17 +1100 (AEDT) Received: from mail.linux-foundation.org (localhost [127.0.0.1]) by mail.linuxfoundation.org (Postfix) with ESMTP id 7C35ED7D; Wed, 27 Mar 2019 17:14:07 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@mail.linuxfoundation.org Received: from smtp1.linuxfoundation.org (smtp1.linux-foundation.org [172.17.192.35]) by mail.linuxfoundation.org (Postfix) with ESMTPS id 6EC68D38 for ; Wed, 27 Mar 2019 17:14:06 +0000 (UTC) X-Greylist: whitelisted by SQLgrey-1.7.6 Received: from mail-pl1-f196.google.com (mail-pl1-f196.google.com [209.85.214.196]) by smtp1.linuxfoundation.org (Postfix) with ESMTPS id 5FF3075B for ; Wed, 27 Mar 2019 17:14:05 +0000 (UTC) Received: by mail-pl1-f196.google.com with SMTP id k2so3612561plt.3 for ; Wed, 27 Mar 2019 10:14:05 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id; bh=u4SQi+LaDk4YpjMeRGuyyVQUioyCk1W7P7gCuOgTLBY=; b=o/qQNBfOLkZgteX9C/eq5WAsXYojOArinIZHB/64EdPZL0NAzkaeKoQk+qqGfBE2r/ UJIL1JTGWQU5rQHvn7E80KHhyUTfvJQGC2HzMaUD4qRRQZMRJ96AP39aogJ5JCpISPkc e0hjyX2Y0bXCyq34dQGjE7NrIuEsU6hK2IZx9CnG7DJnTCR6JhtN51quBeMoxCQi8N5M q3Qg8MvBEoBs6OeMdmonturWeG42E2D9sVQA1jAI+5uLwNfEbbDrX/jXAF+0vXTKbLOA GNjSpskOI0lufWSKsN3vmIykJYBCopxSWV1Mv190f4d93miusXWsY4ozBqwXSzBGaFEu dJkQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id; bh=u4SQi+LaDk4YpjMeRGuyyVQUioyCk1W7P7gCuOgTLBY=; b=QjynIWSbo/AKpLw5ObyChY8B/P1feM1wR3CZzxrYxXOPBSHD5R/HwqMu9JCwa+9c8M Iplgs58GyyKu19apRDvFR1aLHs0SR1yo6RtwkDbyHvGocHwVEKeieWWDatvreVDge8rQ ThVTnXQuYO7dYNYNdZ1ffjAqERlBrTlBi8S7/uFgj1AhsCyixlIeSQOtwv2gPOYHM4dv AWAEFxvgT1YK6ud6Wlv6ZMgupiR0Te/vexzcSyxx3rPyCkyHHT4LUwcyCqgDgbGX0YNS xhr0arR1zUn8YSBrdeggRb2bnQQALPsrmzDtT64F99TjbNuPFtjpvzFmXtIL2d3bEzKG dJNg== X-Gm-Message-State: APjAAAVrbuFwWhMALB3DhKArJwCgdRIVpoysw5hIkrtgGB2xXBZttloq ZEPhoFU/ulZYebaiAN41XAcN/0aq X-Google-Smtp-Source: APXvYqzs50YB59VeSvtmOWtsrsntXkx25LCmR79zs7BvgOb77btnTrRjcG5QhGmpcX1TmCZ6K+G7Jg== X-Received: by 2002:a17:902:be09:: with SMTP id r9mr38019015pls.189.1553706844370; Wed, 27 Mar 2019 10:14:04 -0700 (PDT) Received: from gizo.domain (97-115-83-152.ptld.qwest.net. [97.115.83.152]) by smtp.gmail.com with ESMTPSA id f1sm37034737pgl.35.2019.03.27.10.14.03 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 27 Mar 2019 10:14:03 -0700 (PDT) From: Greg Rose To: dev@openvswitch.org Date: Wed, 27 Mar 2019 10:14:00 -0700 Message-Id: <1553706840-12726-1-git-send-email-gvrose8192@gmail.com> X-Mailer: git-send-email 1.8.3.1 X-Spam-Status: No, score=-1.7 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,FREEMAIL_ENVFROM_END_DIGIT,FREEMAIL_FROM, RCVD_IN_DNSWL_NONE autolearn=no version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on smtp1.linux-foundation.org Cc: "Gustavo A. R. Silva" Subject: [ovs-dev] [PATCH 1/5 V2] datapath: meter: Use struct_size() in kzalloc() X-BeenThere: ovs-dev@openvswitch.org X-Mailman-Version: 2.1.12 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Sender: ovs-dev-bounces@openvswitch.org Errors-To: ovs-dev-bounces@openvswitch.org From: "Gustavo A. R. Silva" Upstream commit: commit c5c3899de09e307e3a0999ab8d620ab0ede05aa1 Author: Gustavo A. R. Silva Date: Tue Jan 15 15:19:17 2019 -0600 openvswitch: meter: Use struct_size() in kzalloc() One of the more common cases of allocation size calculations is finding the size of a structure that has a zero-sized array at the end, along with memory for some number of elements for that array. For example: struct foo { int stuff; struct boo entry[]; }; instance = kzalloc(sizeof(struct foo) + count * sizeof(struct boo), GFP_KERNEL); Instead of leaving these open-coded and prone to type mistakes, we can now use the new struct_size() helper: instance = kzalloc(struct_size(instance, entry, count), GFP_KERNEL); This code was detected with the help of Coccinelle. Signed-off-by: Gustavo A. R. Silva Signed-off-by: David S. Miller Use of struct_size() needed some compat layer adjustments to make use of this new macro. This patch pulls in some of the needed support from the linux mm.h and overflow.h header files. This new header file support is also necessary for the following patch that converts to use of kvmalloc(). Cc: Gustavo A. R. Silva Signed-off-by: Greg Rose --- V2 - Remove accidental change to manpages.mk --- acinclude.m4 | 6 + datapath/linux/Modules.mk | 4 +- datapath/linux/compat/include/linux/mm.h | 44 ++++ datapath/linux/compat/include/linux/overflow.h | 313 +++++++++++++++++++++++++ datapath/meter.c | 4 +- 5 files changed, 368 insertions(+), 3 deletions(-) create mode 100644 datapath/linux/compat/include/linux/mm.h create mode 100644 datapath/linux/compat/include/linux/overflow.h diff --git a/acinclude.m4 b/acinclude.m4 index 3cd6ea7..b1c6798 100644 --- a/acinclude.m4 +++ b/acinclude.m4 @@ -961,6 +961,12 @@ AC_DEFUN([OVS_CHECK_LINUX_COMPAT], [ OVS_FIND_FIELD_IFELSE([$KSRC/include/net/inet_frag.h], [inet_frags], [rnd], [OVS_DEFINE([HAVE_INET_FRAGS_RND])]) + OVS_GREP_IFELSE([$KSRC/include/linux/overflow.h], [__LINUX_OVERFLOW_H], + [OVS_DEFINE([HAVE_OVERFLOW_H])]) + OVS_GREP_IFELSE([$KSRC/include/linux/mm.h], [kvmalloc_array], + [OVS_DEFINE([HAVE_KVMALLOC_ARRAY])]) + OVS_GREP_IFELSE([$KSRC/include/linux/mm.h], [kvmalloc_node], + [OVS_DEFINE([HAVE_KVMALLOC_NODE])]) if cmp -s datapath/linux/kcompat.h.new \ datapath/linux/kcompat.h >/dev/null 2>&1; then diff --git a/datapath/linux/Modules.mk b/datapath/linux/Modules.mk index e31d784..caa2525 100644 --- a/datapath/linux/Modules.mk +++ b/datapath/linux/Modules.mk @@ -112,5 +112,7 @@ openvswitch_headers += \ linux/compat/include/net/netfilter/ipv6/nf_defrag_ipv6.h \ linux/compat/include/net/sctp/checksum.h \ linux/compat/include/net/erspan.h \ - linux/compat/include/uapi/linux/netfilter.h + linux/compat/include/uapi/linux/netfilter.h \ + linux/compat/include/linux/mm.h \ + linux/compat/include/linux/overflow.h EXTRA_DIST += linux/compat/build-aux/export-check-whitelist diff --git a/datapath/linux/compat/include/linux/mm.h b/datapath/linux/compat/include/linux/mm.h new file mode 100644 index 0000000..681f3db --- /dev/null +++ b/datapath/linux/compat/include/linux/mm.h @@ -0,0 +1,44 @@ +#ifndef OVS_MM_H +#define OVS_MM_H + +#include + +#ifndef HAVE_KVMALLOC_ARRAY +#ifndef HAVE_KVMALLOC_NODE +extern void *vmalloc_node(unsigned long size, int node); +#define kvmalloc_node(a, b, c) vmalloc_node(a, c) +#else +extern void *kvmalloc_node(size_t size, gfp_t flags, int node); +#endif /* HAVE_KVMALLOC_NODE */ +static inline void *kvmalloc(size_t size, gfp_t flags) +{ + return kvmalloc_node(size, flags, NUMA_NO_NODE); +} +static inline void *kvzalloc_node(size_t size, gfp_t flags, int node) +{ + return kvmalloc_node(size, flags | __GFP_ZERO, node); +} +static inline void *kvzalloc(size_t size, gfp_t flags) +{ + return kvmalloc(size, flags | __GFP_ZERO); +} + +static inline void *kvmalloc_array(size_t n, size_t size, gfp_t flags) +{ + size_t bytes; + + if (unlikely(check_mul_overflow(n, size, &bytes))) + return NULL; + + return kvmalloc(bytes, flags); +} + +static inline void *kvcalloc(size_t n, size_t size, gfp_t flags) +{ + return kvmalloc_array(n, size, flags | __GFP_ZERO); +} + +#endif +#include_next +#endif /* OVS_MM_H */ + diff --git a/datapath/linux/compat/include/linux/overflow.h b/datapath/linux/compat/include/linux/overflow.h new file mode 100644 index 0000000..ff84356 --- /dev/null +++ b/datapath/linux/compat/include/linux/overflow.h @@ -0,0 +1,313 @@ +/* SPDX-License-Identifier: GPL-2.0 OR MIT */ +#ifdef HAVE_OVERFLOW_H +#include_next +#else +#ifndef __LINUX_OVERFLOW_H +#define __LINUX_OVERFLOW_H + +#include + +/* + * In the fallback code below, we need to compute the minimum and + * maximum values representable in a given type. These macros may also + * be useful elsewhere, so we provide them outside the + * COMPILER_HAS_GENERIC_BUILTIN_OVERFLOW block. + * + * It would seem more obvious to do something like + * + * #define type_min(T) (T)(is_signed_type(T) ? (T)1 << (8*sizeof(T)-1) : 0) + * #define type_max(T) (T)(is_signed_type(T) ? ((T)1 << (8*sizeof(T)-1)) - 1 : ~(T)0) + * + * Unfortunately, the middle expressions, strictly speaking, have + * undefined behaviour, and at least some versions of gcc warn about + * the type_max expression (but not if -fsanitize=undefined is in + * effect; in that case, the warning is deferred to runtime...). + * + * The slightly excessive casting in type_min is to make sure the + * macros also produce sensible values for the exotic type _Bool. [The + * overflow checkers only almost work for _Bool, but that's + * a-feature-not-a-bug, since people shouldn't be doing arithmetic on + * _Bools. Besides, the gcc builtins don't allow _Bool* as third + * argument.] + * + * Idea stolen from + * https://mail-index.netbsd.org/tech-misc/2007/02/05/0000.html - + * credit to Christian Biere. + */ +#define is_signed_type(type) (((type)(-1)) < (type)1) +#define __type_half_max(type) ((type)1 << (8*sizeof(type) - 1 - is_signed_type(type))) +#define type_max(T) ((T)((__type_half_max(T) - 1) + __type_half_max(T))) +#define type_min(T) ((T)((T)-type_max(T)-(T)1)) + + +#ifdef COMPILER_HAS_GENERIC_BUILTIN_OVERFLOW +/* + * For simplicity and code hygiene, the fallback code below insists on + * a, b and *d having the same type (similar to the min() and max() + * macros), whereas gcc's type-generic overflow checkers accept + * different types. Hence we don't just make check_add_overflow an + * alias for __builtin_add_overflow, but add type checks similar to + * below. + */ +#define check_add_overflow(a, b, d) ({ \ + typeof(a) __a = (a); \ + typeof(b) __b = (b); \ + typeof(d) __d = (d); \ + (void) (&__a == &__b); \ + (void) (&__a == __d); \ + __builtin_add_overflow(__a, __b, __d); \ +}) + +#define check_sub_overflow(a, b, d) ({ \ + typeof(a) __a = (a); \ + typeof(b) __b = (b); \ + typeof(d) __d = (d); \ + (void) (&__a == &__b); \ + (void) (&__a == __d); \ + __builtin_sub_overflow(__a, __b, __d); \ +}) + +#define check_mul_overflow(a, b, d) ({ \ + typeof(a) __a = (a); \ + typeof(b) __b = (b); \ + typeof(d) __d = (d); \ + (void) (&__a == &__b); \ + (void) (&__a == __d); \ + __builtin_mul_overflow(__a, __b, __d); \ +}) + +#else + + +/* Checking for unsigned overflow is relatively easy without causing UB. */ +#define __unsigned_add_overflow(a, b, d) ({ \ + typeof(a) __a = (a); \ + typeof(b) __b = (b); \ + typeof(d) __d = (d); \ + (void) (&__a == &__b); \ + (void) (&__a == __d); \ + *__d = __a + __b; \ + *__d < __a; \ +}) +#define __unsigned_sub_overflow(a, b, d) ({ \ + typeof(a) __a = (a); \ + typeof(b) __b = (b); \ + typeof(d) __d = (d); \ + (void) (&__a == &__b); \ + (void) (&__a == __d); \ + *__d = __a - __b; \ + __a < __b; \ +}) +/* + * If one of a or b is a compile-time constant, this avoids a division. + */ +#define __unsigned_mul_overflow(a, b, d) ({ \ + typeof(a) __a = (a); \ + typeof(b) __b = (b); \ + typeof(d) __d = (d); \ + (void) (&__a == &__b); \ + (void) (&__a == __d); \ + *__d = __a * __b; \ + __builtin_constant_p(__b) ? \ + __b > 0 && __a > type_max(typeof(__a)) / __b : \ + __a > 0 && __b > type_max(typeof(__b)) / __a; \ +}) + +/* + * For signed types, detecting overflow is much harder, especially if + * we want to avoid UB. But the interface of these macros is such that + * we must provide a result in *d, and in fact we must produce the + * result promised by gcc's builtins, which is simply the possibly + * wrapped-around value. Fortunately, we can just formally do the + * operations in the widest relevant unsigned type (u64) and then + * truncate the result - gcc is smart enough to generate the same code + * with and without the (u64) casts. + */ + +/* + * Adding two signed integers can overflow only if they have the same + * sign, and overflow has happened iff the result has the opposite + * sign. + */ +#define __signed_add_overflow(a, b, d) ({ \ + typeof(a) __a = (a); \ + typeof(b) __b = (b); \ + typeof(d) __d = (d); \ + (void) (&__a == &__b); \ + (void) (&__a == __d); \ + *__d = (u64)__a + (u64)__b; \ + (((~(__a ^ __b)) & (*__d ^ __a)) \ + & type_min(typeof(__a))) != 0; \ +}) + +/* + * Subtraction is similar, except that overflow can now happen only + * when the signs are opposite. In this case, overflow has happened if + * the result has the opposite sign of a. + */ +#define __signed_sub_overflow(a, b, d) ({ \ + typeof(a) __a = (a); \ + typeof(b) __b = (b); \ + typeof(d) __d = (d); \ + (void) (&__a == &__b); \ + (void) (&__a == __d); \ + *__d = (u64)__a - (u64)__b; \ + ((((__a ^ __b)) & (*__d ^ __a)) \ + & type_min(typeof(__a))) != 0; \ +}) + +/* + * Signed multiplication is rather hard. gcc always follows C99, so + * division is truncated towards 0. This means that we can write the + * overflow check like this: + * + * (a > 0 && (b > MAX/a || b < MIN/a)) || + * (a < -1 && (b > MIN/a || b < MAX/a) || + * (a == -1 && b == MIN) + * + * The redundant casts of -1 are to silence an annoying -Wtype-limits + * (included in -Wextra) warning: When the type is u8 or u16, the + * __b_c_e in check_mul_overflow obviously selects + * __unsigned_mul_overflow, but unfortunately gcc still parses this + * code and warns about the limited range of __b. + */ + +#define __signed_mul_overflow(a, b, d) ({ \ + typeof(a) __a = (a); \ + typeof(b) __b = (b); \ + typeof(d) __d = (d); \ + typeof(a) __tmax = type_max(typeof(a)); \ + typeof(a) __tmin = type_min(typeof(a)); \ + (void) (&__a == &__b); \ + (void) (&__a == __d); \ + *__d = (u64)__a * (u64)__b; \ + (__b > 0 && (__a > __tmax/__b || __a < __tmin/__b)) || \ + (__b < (typeof(__b))-1 && (__a > __tmin/__b || __a < __tmax/__b)) || \ + (__b == (typeof(__b))-1 && __a == __tmin); \ +}) + + +#define check_add_overflow(a, b, d) \ + __builtin_choose_expr(is_signed_type(typeof(a)), \ + __signed_add_overflow(a, b, d), \ + __unsigned_add_overflow(a, b, d)) + +#define check_sub_overflow(a, b, d) \ + __builtin_choose_expr(is_signed_type(typeof(a)), \ + __signed_sub_overflow(a, b, d), \ + __unsigned_sub_overflow(a, b, d)) + +#define check_mul_overflow(a, b, d) \ + __builtin_choose_expr(is_signed_type(typeof(a)), \ + __signed_mul_overflow(a, b, d), \ + __unsigned_mul_overflow(a, b, d)) + + +#endif /* COMPILER_HAS_GENERIC_BUILTIN_OVERFLOW */ + +/** check_shl_overflow() - Calculate a left-shifted value and check overflow + * + * @a: Value to be shifted + * @s: How many bits left to shift + * @d: Pointer to where to store the result + * + * Computes *@d = (@a << @s) + * + * Returns true if '*d' cannot hold the result or when 'a << s' doesn't + * make sense. Example conditions: + * - 'a << s' causes bits to be lost when stored in *d. + * - 's' is garbage (e.g. negative) or so large that the result of + * 'a << s' is guaranteed to be 0. + * - 'a' is negative. + * - 'a << s' sets the sign bit, if any, in '*d'. + * + * '*d' will hold the results of the attempted shift, but is not + * considered "safe for use" if false is returned. + */ +#define check_shl_overflow(a, s, d) ({ \ + typeof(a) _a = a; \ + typeof(s) _s = s; \ + typeof(d) _d = d; \ + u64 _a_full = _a; \ + unsigned int _to_shift = \ + _s >= 0 && _s < 8 * sizeof(*d) ? _s : 0; \ + *_d = (_a_full << _to_shift); \ + (_to_shift != _s || *_d < 0 || _a < 0 || \ + (*_d >> _to_shift) != _a); \ +}) + +/** + * array_size() - Calculate size of 2-dimensional array. + * + * @a: dimension one + * @b: dimension two + * + * Calculates size of 2-dimensional array: @a * @b. + * + * Returns: number of bytes needed to represent the array or SIZE_MAX on + * overflow. + */ +static inline __must_check size_t array_size(size_t a, size_t b) +{ + size_t bytes; + + if (check_mul_overflow(a, b, &bytes)) + return SIZE_MAX; + + return bytes; +} + +/** + * array3_size() - Calculate size of 3-dimensional array. + * + * @a: dimension one + * @b: dimension two + * @c: dimension three + * + * Calculates size of 3-dimensional array: @a * @b * @c. + * + * Returns: number of bytes needed to represent the array or SIZE_MAX on + * overflow. + */ +static inline __must_check size_t array3_size(size_t a, size_t b, size_t c) +{ + size_t bytes; + + if (check_mul_overflow(a, b, &bytes)) + return SIZE_MAX; + if (check_mul_overflow(bytes, c, &bytes)) + return SIZE_MAX; + + return bytes; +} + +static inline __must_check size_t __ab_c_size(size_t n, size_t size, size_t c) +{ + size_t bytes; + + if (check_mul_overflow(n, size, &bytes)) + return SIZE_MAX; + if (check_add_overflow(bytes, c, &bytes)) + return SIZE_MAX; + + return bytes; +} + +/** + * struct_size() - Calculate size of structure with trailing array. + * @p: Pointer to the structure. + * @member: Name of the array member. + * @n: Number of elements in the array. + * + * Calculates size of memory needed for structure @p followed by an + * array of @n @member elements. + * + * Return: number of bytes needed or SIZE_MAX on overflow. + */ +#define struct_size(p, member, n) \ + __ab_c_size(n, \ + sizeof(*(p)->member) + __must_be_array((p)->member),\ + sizeof(*(p))) + +#endif /* __LINUX_OVERFLOW_H */ +#endif /* HAVE_OVERFLOW_H */ diff --git a/datapath/meter.c b/datapath/meter.c index 281d869..51ec149 100644 --- a/datapath/meter.c +++ b/datapath/meter.c @@ -18,6 +18,7 @@ #include #include +#include #include "datapath.h" #include "meter.h" @@ -216,8 +217,7 @@ static struct dp_meter *dp_meter_create(struct nlattr **a) return ERR_PTR(-EINVAL); /* Allocate and set up the meter before locking anything. */ - meter = kzalloc(n_bands * sizeof(struct dp_meter_band) + - sizeof(*meter), GFP_KERNEL); + meter = kzalloc(struct_size(meter, bands, n_bands), GFP_KERNEL); if (!meter) return ERR_PTR(-ENOMEM);