LU-17000 gss: Fix Out-of-bounds access under svcgssd_proc.c

Problem reported by coverity was passing 32bit type and
then dereferencing to larger 64bit under function
handle_channel_request(). This patch address this issue.

Since this is an uapi and to catch corner cases like
kernel modules being updated separately from user tools
RSI_DOWNCALL_MAGIC is also changed from 0x6d6dd62a to
0x6d6dd63a.

This patch also changes 32bit member (sid_hash) of
'struct rsi_downcall_data' to 64bit. Which also requires
changing of wiretest.c and wirecheck.c

Lustre-change: https://review.whamcloud.com/52920
Lustre-commit: 7d764f1f11be144ad26e33aa8cecedc5bb708793

CoverityID: 404758 ("Out-of-bounds access")
Fixes: 4daf43ac3c ("LU-17015 gss: support large kerberos token for rpc sec init")
Test-Parameters: kerberos=true testlist=sanity-krb5
Signed-off-by: Arshad Hussain <arshad.hussain@aeoncomputing.com>
Change-Id: I8041cd4063f1b1cefdebf5681df426be61820f99
Reviewed-by: Andreas Dilger <adilger@whamcloud.com>
Reviewed-by: Sebastien Buisson <sbuisson@ddn.com>
Reviewed-on: https://review.whamcloud.com/c/ex/lustre-release/+/53440
Tested-by: jenkins <devops@whamcloud.com>
Tested-by: Maloo <maloo@whamcloud.com>

diff --git a/lustre/include/uapi/linux/lustre/lgss.h b/lustre/include/uapi/linux/lustre/lgss.h
index 154d678..d353fba 100644 (file)
--- a/lustre/include/uapi/linux/lustre/lgss.h
+++ b/lustre/include/uapi/linux/lustre/lgss.h
@@ -72,18 +72,27 @@ struct lgssd_ioctl_param {
 };
 
 #define GSS_SOCKET_PATH        "/tmp/svcgssd.socket"
-#define RSI_DOWNCALL_MAGIC     0x6d6dd62a
+/*
+ * Old RSI_DOWNCALL_MAGIC was:
+ * #define RSI_DOWNCALL_MAGIC  0x6d6dd62a
+ *
+ * This is an uapi and to catch cases like kernel modules
+ * being updated separately from user tools new
+ * RSI_DOWNCALL_MAGIC(0x6d6dd63a) was introduced
+ */
+#define RSI_DOWNCALL_MAGIC     0x6d6dd63a
 #define RSI_DOWNCALL_PATH      "sptlrpc/gss/rsi_info"
 #define RSI_CACHE_NAME         "rsicache"
 
 struct rsi_downcall_data {
        __u32     sid_magic;
        __u32     sid_err;
-       __u32     sid_hash;
+       __u32     sid_unused;
        __u32     sid_maj_stat;
        __u32     sid_min_stat;
        __u32     sid_len;
        __s64     sid_offset;
+       __u64     sid_hash;
        /* sid_val contains in_handle, in_token,
         * out_handle, out_token
         */

diff --git a/lustre/ptlrpc/wiretest.c b/lustre/ptlrpc/wiretest.c
index d1d5947..4f10a37 100644 (file)
--- a/lustre/ptlrpc/wiretest.c
+++ b/lustre/ptlrpc/wiretest.c
@@ -4484,7 +4484,7 @@ void lustre_assert_wire_constants(void)
                 (long long)(int)sizeof(((struct llog_changelog_user_rec *)0)->cur_tail));
 
        /* Checks for struct rsi_downcall_data */
-       LASSERTF((int)sizeof(struct rsi_downcall_data) == 32, "found %lld\n",
+       LASSERTF((int)sizeof(struct rsi_downcall_data) == 40, "found %lld\n",
                 (long long)(int)sizeof(struct rsi_downcall_data));
        LASSERTF((int)offsetof(struct rsi_downcall_data, sid_magic) == 0, "found %lld\n",
                 (long long)(int)offsetof(struct rsi_downcall_data, sid_magic));
@@ -4494,10 +4494,10 @@ void lustre_assert_wire_constants(void)
                 (long long)(int)offsetof(struct rsi_downcall_data, sid_err));
        LASSERTF((int)sizeof(((struct rsi_downcall_data *)0)->sid_err) == 4, "found %lld\n",
                 (long long)(int)sizeof(((struct rsi_downcall_data *)0)->sid_err));
-       LASSERTF((int)offsetof(struct rsi_downcall_data, sid_hash) == 8, "found %lld\n",
-                (long long)(int)offsetof(struct rsi_downcall_data, sid_hash));
-       LASSERTF((int)sizeof(((struct rsi_downcall_data *)0)->sid_hash) == 4, "found %lld\n",
-                (long long)(int)sizeof(((struct rsi_downcall_data *)0)->sid_hash));
+       LASSERTF((int)offsetof(struct rsi_downcall_data, sid_unused) == 8, "found %lld\n",
+                (long long)(int)offsetof(struct rsi_downcall_data, sid_unused));
+       LASSERTF((int)sizeof(((struct rsi_downcall_data *)0)->sid_unused) == 4, "found %lld\n",
+                (long long)(int)sizeof(((struct rsi_downcall_data *)0)->sid_unused));
        LASSERTF((int)offsetof(struct rsi_downcall_data, sid_maj_stat) == 12, "found %lld\n",
                 (long long)(int)offsetof(struct rsi_downcall_data, sid_maj_stat));
        LASSERTF((int)sizeof(((struct rsi_downcall_data *)0)->sid_maj_stat) == 4, "found %lld\n",
@@ -4514,7 +4514,11 @@ void lustre_assert_wire_constants(void)
                 (long long)(int)offsetof(struct rsi_downcall_data, sid_offset));
        LASSERTF((int)sizeof(((struct rsi_downcall_data *)0)->sid_offset) == 8, "found %lld\n",
                 (long long)(int)sizeof(((struct rsi_downcall_data *)0)->sid_offset));
-       LASSERTF((int)offsetof(struct rsi_downcall_data, sid_val) == 32, "found %lld\n",
+       LASSERTF((int)offsetof(struct rsi_downcall_data, sid_hash) == 32, "found %lld\n",
+                (long long)(int)offsetof(struct rsi_downcall_data, sid_hash));
+       LASSERTF((int)sizeof(((struct rsi_downcall_data *)0)->sid_hash) == 8, "found %lld\n",
+                (long long)(int)sizeof(((struct rsi_downcall_data *)0)->sid_hash));
+       LASSERTF((int)offsetof(struct rsi_downcall_data, sid_val) == 40, "found %lld\n",
                 (long long)(int)offsetof(struct rsi_downcall_data, sid_val));
        LASSERTF((int)sizeof(((struct rsi_downcall_data *)0)->sid_val) == 0, "found %lld\n",
                 (long long)(int)sizeof(((struct rsi_downcall_data *)0)->sid_val));

diff --git a/lustre/utils/gss/svcgssd_proc.c b/lustre/utils/gss/svcgssd_proc.c
index fa61a74..456e19d 100644 (file)
--- a/lustre/utils/gss/svcgssd_proc.c
+++ b/lustre/utils/gss/svcgssd_proc.c
@@ -177,7 +177,7 @@ out:
 
 #define RPCSEC_GSS_SEQ_WIN     5
 
-static int send_response(int auth_res, uint64_t hash,
+static int send_response(int auth_res, __u64 hash,
                        gss_buffer_desc *in_handle, gss_buffer_desc *in_token,
                        u_int32_t maj_stat, u_int32_t min_stat,
                        gss_buffer_desc *out_handle, gss_buffer_desc *out_token)
@@ -912,7 +912,8 @@ int handle_channel_request(int fd)
                .maj_stat               = GSS_S_FAILURE,
                .ctx                    = GSS_C_NO_CONTEXT,
        };
-       uint64_t hash = 0;
+       __u64 hash = 0;
+       __u64 tmp_lustre_svc = 0;
 
        printerr(LL_INFO, "handling request\n");
        if (readline(fd, &lbuf, &lbuflen) != 1) {
@@ -923,16 +924,17 @@ int handle_channel_request(int fd)
        cp = lbuf;
 
        /* see rsi_do_upcall() for the format of data being input here */
-       rc = gss_u64_read_string(&cp, (__u64 *)&hash);
+       rc = gss_u64_read_string(&cp, &hash);
        if (rc < 0) {
                printerr(LL_ERR, "ERROR: failed parsing request: hash\n");
                goto out_err;
        }
-       rc = gss_u64_read_string(&cp, (__u64 *)&snd.lustre_svc);
+       rc = gss_u64_read_string(&cp, &tmp_lustre_svc);
        if (rc < 0) {
                printerr(LL_ERR, "ERROR: failed parsing request: lustre svc\n");
                goto out_err;
        }
+       snd.lustre_svc = tmp_lustre_svc;
        /* lustre_svc is the svc and gss subflavor */
        lustre_mech = (snd.lustre_svc & LUSTRE_GSS_MECH_MASK) >>
                LUSTRE_GSS_MECH_SHIFT;

diff --git a/lustre/utils/wirecheck.c b/lustre/utils/wirecheck.c
index ce1f805..7d2a380 100644 (file)
--- a/lustre/utils/wirecheck.c
+++ b/lustre/utils/wirecheck.c
@@ -2032,11 +2032,12 @@ check_rsi_downcall_data(void)
        CHECK_STRUCT(rsi_downcall_data);
        CHECK_MEMBER(rsi_downcall_data, sid_magic);
        CHECK_MEMBER(rsi_downcall_data, sid_err);
-       CHECK_MEMBER(rsi_downcall_data, sid_hash);
+       CHECK_MEMBER(rsi_downcall_data, sid_unused);
        CHECK_MEMBER(rsi_downcall_data, sid_maj_stat);
        CHECK_MEMBER(rsi_downcall_data, sid_min_stat);
        CHECK_MEMBER(rsi_downcall_data, sid_len);
        CHECK_MEMBER(rsi_downcall_data, sid_offset);
+       CHECK_MEMBER(rsi_downcall_data, sid_hash);
        CHECK_MEMBER(rsi_downcall_data, sid_val);
 }
 

diff --git a/lustre/utils/wiretest.c b/lustre/utils/wiretest.c
index 155abc2..a987dcd 100644 (file)
--- a/lustre/utils/wiretest.c
+++ b/lustre/utils/wiretest.c
@@ -4520,7 +4520,7 @@ void lustre_assert_wire_constants(void)
                 (long long)(int)sizeof(((struct llog_changelog_user_rec *)0)->cur_tail));
 
        /* Checks for struct rsi_downcall_data */
-       LASSERTF((int)sizeof(struct rsi_downcall_data) == 32, "found %lld\n",
+       LASSERTF((int)sizeof(struct rsi_downcall_data) == 40, "found %lld\n",
                 (long long)(int)sizeof(struct rsi_downcall_data));
        LASSERTF((int)offsetof(struct rsi_downcall_data, sid_magic) == 0, "found %lld\n",
                 (long long)(int)offsetof(struct rsi_downcall_data, sid_magic));
@@ -4530,10 +4530,10 @@ void lustre_assert_wire_constants(void)
                 (long long)(int)offsetof(struct rsi_downcall_data, sid_err));
        LASSERTF((int)sizeof(((struct rsi_downcall_data *)0)->sid_err) == 4, "found %lld\n",
                 (long long)(int)sizeof(((struct rsi_downcall_data *)0)->sid_err));
-       LASSERTF((int)offsetof(struct rsi_downcall_data, sid_hash) == 8, "found %lld\n",
-                (long long)(int)offsetof(struct rsi_downcall_data, sid_hash));
-       LASSERTF((int)sizeof(((struct rsi_downcall_data *)0)->sid_hash) == 4, "found %lld\n",
-                (long long)(int)sizeof(((struct rsi_downcall_data *)0)->sid_hash));
+       LASSERTF((int)offsetof(struct rsi_downcall_data, sid_unused) == 8, "found %lld\n",
+                (long long)(int)offsetof(struct rsi_downcall_data, sid_unused));
+       LASSERTF((int)sizeof(((struct rsi_downcall_data *)0)->sid_unused) == 4, "found %lld\n",
+                (long long)(int)sizeof(((struct rsi_downcall_data *)0)->sid_unused));
        LASSERTF((int)offsetof(struct rsi_downcall_data, sid_maj_stat) == 12, "found %lld\n",
                 (long long)(int)offsetof(struct rsi_downcall_data, sid_maj_stat));
        LASSERTF((int)sizeof(((struct rsi_downcall_data *)0)->sid_maj_stat) == 4, "found %lld\n",
@@ -4550,7 +4550,11 @@ void lustre_assert_wire_constants(void)
                 (long long)(int)offsetof(struct rsi_downcall_data, sid_offset));
        LASSERTF((int)sizeof(((struct rsi_downcall_data *)0)->sid_offset) == 8, "found %lld\n",
                 (long long)(int)sizeof(((struct rsi_downcall_data *)0)->sid_offset));
-       LASSERTF((int)offsetof(struct rsi_downcall_data, sid_val) == 32, "found %lld\n",
+       LASSERTF((int)offsetof(struct rsi_downcall_data, sid_hash) == 32, "found %lld\n",
+                (long long)(int)offsetof(struct rsi_downcall_data, sid_hash));
+       LASSERTF((int)sizeof(((struct rsi_downcall_data *)0)->sid_hash) == 8, "found %lld\n",
+                (long long)(int)sizeof(((struct rsi_downcall_data *)0)->sid_hash));
+       LASSERTF((int)offsetof(struct rsi_downcall_data, sid_val) == 40, "found %lld\n",
                 (long long)(int)offsetof(struct rsi_downcall_data, sid_val));
        LASSERTF((int)sizeof(((struct rsi_downcall_data *)0)->sid_val) == 0, "found %lld\n",
                 (long long)(int)sizeof(((struct rsi_downcall_data *)0)->sid_val));