IB/core: Split uverbs_get_const/default to consider target type
authorYishai Hadas <yishaih@nvidia.com>
Thu, 4 Mar 2021 13:05:00 +0000 (15:05 +0200)
committerJason Gunthorpe <jgg@nvidia.com>
Fri, 12 Mar 2021 00:20:36 +0000 (20:20 -0400)
Change uverbs_get_const/uverbs_get_const_default to work properly with
both signed/unsigned parameters.

Current APIs mix s64 and u64 which leads to incorrect check when u64
value was supplied and its upper bit was set. In that case
uverbs_get_const() / uverbs_get_const_default() lower bound check may
fail unexpectedly, target is unsigned (lower bound is 0) but value
became negative as of the s64 usage.

Split to have two different APIs, no change to callers as the required
API will be called internally according to the target type.

Link: https://lore.kernel.org/r/20210304130501.1102577-3-leon@kernel.org
Signed-off-by: Yishai Hadas <yishaih@nvidia.com>
Signed-off-by: Leon Romanovsky <leonro@nvidia.com>
Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
drivers/infiniband/core/uverbs_ioctl.c
drivers/infiniband/hw/mlx5/main.c
include/rdma/uverbs_ioctl.h

index ff047eb024ab1a7549ff2eb8b2daaebcf13bf646..990f0724acc6b660ef2dfc2adeeb00b510e191f3 100644 (file)
@@ -752,9 +752,10 @@ int uverbs_output_written(const struct uverbs_attr_bundle *bundle, size_t idx)
        return uverbs_set_output(bundle, attr);
 }
 
-int _uverbs_get_const(s64 *to, const struct uverbs_attr_bundle *attrs_bundle,
-                     size_t idx, s64 lower_bound, u64 upper_bound,
-                     s64  *def_val)
+int _uverbs_get_const_signed(s64 *to,
+                            const struct uverbs_attr_bundle *attrs_bundle,
+                            size_t idx, s64 lower_bound, u64 upper_bound,
+                            s64  *def_val)
 {
        const struct uverbs_attr *attr;
 
@@ -773,7 +774,30 @@ int _uverbs_get_const(s64 *to, const struct uverbs_attr_bundle *attrs_bundle,
 
        return 0;
 }
-EXPORT_SYMBOL(_uverbs_get_const);
+EXPORT_SYMBOL(_uverbs_get_const_signed);
+
+int _uverbs_get_const_unsigned(u64 *to,
+                              const struct uverbs_attr_bundle *attrs_bundle,
+                              size_t idx, u64 upper_bound, u64 *def_val)
+{
+       const struct uverbs_attr *attr;
+
+       attr = uverbs_attr_get(attrs_bundle, idx);
+       if (IS_ERR(attr)) {
+               if ((PTR_ERR(attr) != -ENOENT) || !def_val)
+                       return PTR_ERR(attr);
+
+               *to = *def_val;
+       } else {
+               *to = attr->ptr_attr.data;
+       }
+
+       if (*to > upper_bound)
+               return -EINVAL;
+
+       return 0;
+}
+EXPORT_SYMBOL(_uverbs_get_const_unsigned);
 
 int uverbs_copy_to_struct_or_zero(const struct uverbs_attr_bundle *bundle,
                                  size_t idx, const void *from, size_t size)
index 4be7bccefaa40a5db5740ec6fb77b60e2f69bca5..9ff28f778c0e13eca5db33b7bfdfd94a66b56547 100644 (file)
@@ -42,6 +42,7 @@
 #include "counters.h"
 #include <linux/mlx5/accel.h>
 #include <rdma/uverbs_std_types.h>
+#include <rdma/uverbs_ioctl.h>
 #include <rdma/mlx5_user_ioctl_verbs.h>
 #include <rdma/mlx5_user_ioctl_cmds.h>
 #include <rdma/ib_umem_odp.h>
index 39ef204753ec16b1a51c0c4fd69c1a3f60d2d176..3829b6ef4bb68d632b68543022c87d46b25ef587 100644 (file)
@@ -875,9 +875,14 @@ static inline __malloc void *uverbs_kcalloc(struct uverbs_attr_bundle *bundle,
                return ERR_PTR(-EOVERFLOW);
        return uverbs_zalloc(bundle, bytes);
 }
-int _uverbs_get_const(s64 *to, const struct uverbs_attr_bundle *attrs_bundle,
-                     size_t idx, s64 lower_bound, u64 upper_bound,
-                     s64 *def_val);
+
+int _uverbs_get_const_signed(s64 *to,
+                            const struct uverbs_attr_bundle *attrs_bundle,
+                            size_t idx, s64 lower_bound, u64 upper_bound,
+                            s64 *def_val);
+int _uverbs_get_const_unsigned(u64 *to,
+                              const struct uverbs_attr_bundle *attrs_bundle,
+                              size_t idx, u64 upper_bound, u64 *def_val);
 int uverbs_copy_to_struct_or_zero(const struct uverbs_attr_bundle *bundle,
                                  size_t idx, const void *from, size_t size);
 #else
@@ -921,27 +926,76 @@ uverbs_copy_to_struct_or_zero(const struct uverbs_attr_bundle *bundle,
 {
        return -EINVAL;
 }
+static int
+_uverbs_get_const_signed(s64 *to, const struct uverbs_attr_bundle *attrs_bundle,
+                        size_t idx, s64 lower_bound, u64 upper_bound,
+                        s64 *def_val)
+{
+       return -EINVAL;
+}
+static int
+_uverbs_get_const_unsigned(u64 *to,
+                          const struct uverbs_attr_bundle *attrs_bundle,
+                          size_t idx, u64 upper_bound, u64 *def_val)
+{
+       return -EINVAL;
+}
 #endif
 
-#define uverbs_get_const(_to, _attrs_bundle, _idx)                             \
+#define uverbs_get_const_signed(_to, _attrs_bundle, _idx)                      \
        ({                                                                     \
                s64 _val;                                                      \
-               int _ret = _uverbs_get_const(&_val, _attrs_bundle, _idx,       \
-                                            type_min(typeof(*_to)),           \
-                                            type_max(typeof(*_to)), NULL);    \
-               (*_to) = _val;                                                 \
+               int _ret =                                                     \
+                       _uverbs_get_const_signed(&_val, _attrs_bundle, _idx,   \
+                                         type_min(typeof(*(_to))),            \
+                                         type_max(typeof(*(_to))), NULL);     \
+               (*(_to)) = _val;                                               \
                _ret;                                                          \
        })
 
-#define uverbs_get_const_default(_to, _attrs_bundle, _idx, _default)           \
+#define uverbs_get_const_unsigned(_to, _attrs_bundle, _idx)                    \
+       ({                                                                     \
+               u64 _val;                                                      \
+               int _ret =                                                     \
+                       _uverbs_get_const_unsigned(&_val, _attrs_bundle, _idx, \
+                                         type_max(typeof(*(_to))), NULL);     \
+               (*(_to)) = _val;                                               \
+               _ret;                                                          \
+       })
+
+#define uverbs_get_const_default_signed(_to, _attrs_bundle, _idx, _default)    \
        ({                                                                     \
                s64 _val;                                                      \
                s64 _def_val = _default;                                       \
                int _ret =                                                     \
-                       _uverbs_get_const(&_val, _attrs_bundle, _idx,          \
-                                         type_min(typeof(*_to)),              \
-                                         type_max(typeof(*_to)), &_def_val);  \
-               (*_to) = _val;                                                 \
+                       _uverbs_get_const_signed(&_val, _attrs_bundle, _idx,   \
+                               type_min(typeof(*(_to))),                      \
+                               type_max(typeof(*(_to))), &_def_val);          \
+               (*(_to)) = _val;                                               \
+               _ret;                                                          \
+       })
+
+#define uverbs_get_const_default_unsigned(_to, _attrs_bundle, _idx, _default)  \
+       ({                                                                     \
+               u64 _val;                                                      \
+               u64 _def_val = _default;                                       \
+               int _ret =                                                     \
+                       _uverbs_get_const_unsigned(&_val, _attrs_bundle, _idx, \
+                               type_max(typeof(*(_to))), &_def_val);          \
+               (*(_to)) = _val;                                               \
                _ret;                                                          \
        })
+
+#define uverbs_get_const(_to, _attrs_bundle, _idx)                             \
+       (is_signed_type(typeof(*(_to))) ?                                      \
+                uverbs_get_const_signed(_to, _attrs_bundle, _idx) :           \
+                uverbs_get_const_unsigned(_to, _attrs_bundle, _idx))          \
+
+#define uverbs_get_const_default(_to, _attrs_bundle, _idx, _default)           \
+       (is_signed_type(typeof(*(_to))) ?                                      \
+                uverbs_get_const_default_signed(_to, _attrs_bundle, _idx,     \
+                                                 _default) :                  \
+                uverbs_get_const_default_unsigned(_to, _attrs_bundle, _idx,   \
+                                                   _default))
+
 #endif