LU-17504 build: fix lock_handle array-index-out-of-bounds

After Linux kernel patch "ubsan: Tighten UBSAN_BOUNDS on GCC"
(commit v6.4-rc2-1-g2d47c6956ab3), flexible trailing arrays
declared like 'lock_handle[2]' will generate warnings when
CONFIG_UBSAN & co. is enabled:

    UBSAN: array-index-out-of-bounds in ldlm_request.c:1282:18
    index 2 is out of range for type 'lustre_handle [2]'

The declaration lock_handle[LDLM_LOCKREQ_HANDLES] confuses the
compiler into thinking there are only two fields in lock_handle,
but the caller often allocates extra fields beyond this for more
locks to be cancelled due to Early Lock Cancellation or from LRU.

Rather than have a second flexible array after lustre_handle[2],
declare the whole array as flexible, and fix up the few sites
that are allocating this array to ensure LDLM_LOCKREQ_HANDLES
fields are allocated at a minimum.

This subtly changes the checks in wiretest.c due to the removal
of the 2 "base" handles in ldlm_request, but I believe this is not
changing the wire protocol because it still allocates those handles
directly, and I have verified interoperability with a 2.14.0 server.

Test-Parameters: testlist=runtests clientversion=2.14
Test-Parameters: testlist=runtests serverversion=2.14
Test-Parameters: testlist=runtests clientversion=2.15
Test-Parameters: testlist=runtests serverversion=2.15
Test-Parameters: testlist=runtests clientversion=EXA5
Test-Parameters: testlist=runtests serverversion=EXA5
Test-Parameters: testlist=runtests clientversion=EXA6
Test-Parameters: testlist=runtests serverversion=EXA6
Signed-off-by: Andreas Dilger <adilger@whamcloud.com>
Change-Id: I9695fb44f1b5c84bb750d2983cdd8b939e3ebbe5
Reviewed-on: https://review.whamcloud.com/c/fs/lustre-release/+/54926
Tested-by: jenkins <devops@whamcloud.com>
Tested-by: Maloo <maloo@whamcloud.com>
Reviewed-by: Jian Yu <yujian@whamcloud.com>
Reviewed-by: Shaun Tancheff <shaun.tancheff@hpe.com>
Reviewed-by: Yang Sheng <ys@whamcloud.com>
Reviewed-by: James Simmons <jsimmons@infradead.org>
Reviewed-by: Oleg Drokin <green@whamcloud.com>

diff --git a/lustre/include/uapi/linux/lustre/lustre_idl.h b/lustre/include/uapi/linux/lustre/lustre_idl.h
index 8be4759..bc88bd1 100644 (file)
--- a/lustre/include/uapi/linux/lustre/lustre_idl.h
+++ b/lustre/include/uapi/linux/lustre/lustre_idl.h
@@ -2636,8 +2636,8 @@ struct ldlm_lock_desc {
 struct ldlm_request {
        __u32                   lock_flags; /* LDLM_FL_*,(lustre_dlm_flags.h) */
        __u32                   lock_count; /* Num of locks in lock_handle[] */
-       struct ldlm_lock_desc   lock_desc; /* lock descriptor */
-       struct lustre_handle    lock_handle[LDLM_LOCKREQ_HANDLES];
+       struct ldlm_lock_desc   lock_desc;  /* lock descriptor */
+       struct lustre_handle    lock_handle[]; /* was LDLM_LOCKREQ_HANDLES */
 };
 
 struct ldlm_reply {

diff --git a/lustre/ldlm/ldlm_lockd.c b/lustre/ldlm/ldlm_lockd.c
index d7a1944..ca59464 100644 (file)
--- a/lustre/ldlm/ldlm_lockd.c
+++ b/lustre/ldlm/ldlm_lockd.c
@@ -1769,9 +1769,8 @@ int ldlm_request_cancel(struct ptlrpc_request *req,
        ENTRY;
 
        size = req_capsule_get_size(&req->rq_pill, &RMF_DLM_REQ, RCL_CLIENT);
-       if (size <= offsetof(struct ldlm_request, lock_handle) ||
-           (size - offsetof(struct ldlm_request, lock_handle)) /
-            sizeof(struct lustre_handle) < dlm_req->lock_count)
+       if (size <= sizeof(*dlm_req) || dlm_req->lock_count >
+           (size - sizeof(*dlm_req)) / sizeof(struct lustre_handle))
                RETURN(0);
 
        count = dlm_req->lock_count ? dlm_req->lock_count : 1;

diff --git a/lustre/ldlm/ldlm_request.c b/lustre/ldlm/ldlm_request.c
index e51cd98..8ae1c88 100644 (file)
--- a/lustre/ldlm/ldlm_request.c
+++ b/lustre/ldlm/ldlm_request.c
@@ -88,24 +88,21 @@ struct ldlm_async_args {
  * LDLM_LOCKREQ_HANDLE -1 slots are available.
  * Otherwise, LDLM_LOCKREQ_HANDLE slots are available.
  *
- * \param[in] count
- * \param[in] type
+ * \param[in] count - total number of lock handles to include for cancel
+ * \param[in] type  - LDLM RPC request type
  *
  * \retval size of the request buffer
  */
 static int ldlm_request_bufsize(int count, int type)
 {
-       int avail = LDLM_LOCKREQ_HANDLES;
-
        if (type == LDLM_ENQUEUE)
-               avail -= LDLM_ENQUEUE_CANCEL_OFF;
+               count++;
 
-       if (count > avail)
-               avail = (count - avail) * sizeof(struct lustre_handle);
-       else
-               avail = 0;
+       /* keep minimum handles to keep struct size for compatibility */
+       if (count < LDLM_LOCKREQ_HANDLES)
+               count = LDLM_LOCKREQ_HANDLES;
 
-       return sizeof(struct ldlm_request) + avail;
+       return offsetof(struct ldlm_request, lock_handle[count]);
 }
 
 static void ldlm_expired_completion_wait(struct lock_wait_data *lwd)

diff --git a/lustre/ptlrpc/layout.c b/lustre/ptlrpc/layout.c
index 4fc1c3c..42e0fa3 100644 (file)
--- a/lustre/ptlrpc/layout.c
+++ b/lustre/ptlrpc/layout.c
@@ -1147,7 +1147,8 @@ EXPORT_SYMBOL(RMF_CONNECT_DATA);
 
 struct req_msg_field RMF_DLM_REQ =
        DEFINE_MSGF("dlm_req", RMF_F_NO_SIZE_CHECK /* ldlm_request_bufsize */,
-                   sizeof(struct ldlm_request),
+                   offsetof(struct ldlm_request,
+                            lock_handle[LDLM_LOCKREQ_HANDLES]),
                    lustre_swab_ldlm_request, NULL);
 EXPORT_SYMBOL(RMF_DLM_REQ);
 

diff --git a/lustre/ptlrpc/wiretest.c b/lustre/ptlrpc/wiretest.c
index c85dc3b..b37eee1 100644 (file)
--- a/lustre/ptlrpc/wiretest.c
+++ b/lustre/ptlrpc/wiretest.c
@@ -3847,7 +3847,7 @@ void lustre_assert_wire_constants(void)
                 (long long)(int)sizeof(((struct ldlm_lock_desc *)0)->l_policy_data));
 
        /* Checks for struct ldlm_request */
-       LASSERTF((int)sizeof(struct ldlm_request) == 104, "found %lld\n",
+       LASSERTF((int)sizeof(struct ldlm_request) == 88, "found %lld\n",
                 (long long)(int)sizeof(struct ldlm_request));
        LASSERTF((int)offsetof(struct ldlm_request, lock_flags) == 0, "found %lld\n",
                 (long long)(int)offsetof(struct ldlm_request, lock_flags));
@@ -3863,8 +3863,9 @@ void lustre_assert_wire_constants(void)
                 (long long)(int)sizeof(((struct ldlm_request *)0)->lock_desc));
        LASSERTF((int)offsetof(struct ldlm_request, lock_handle) == 88, "found %lld\n",
                 (long long)(int)offsetof(struct ldlm_request, lock_handle));
-       LASSERTF((int)sizeof(((struct ldlm_request *)0)->lock_handle) == 16, "found %lld\n",
-                (long long)(int)sizeof(((struct ldlm_request *)0)->lock_handle));
+       LASSERTF((int)sizeof(*((struct ldlm_request *)0)->lock_handle) == 8, "found %lld\n",
+                (long long)(int)sizeof(*((struct ldlm_request *)0)->lock_handle));
+       BUILD_BUG_ON(offsetof(struct ldlm_request, lock_handle) != sizeof(struct ldlm_request));
 
        /* Checks for struct ldlm_reply */
        LASSERTF((int)sizeof(struct ldlm_reply) == 112, "found %lld\n",

diff --git a/lustre/utils/wirecheck.c b/lustre/utils/wirecheck.c
index e427c56..994df70 100644 (file)
--- a/lustre/utils/wirecheck.c
+++ b/lustre/utils/wirecheck.c
@@ -1725,7 +1725,7 @@ check_ldlm_request(void)
        CHECK_MEMBER(ldlm_request, lock_flags);
        CHECK_MEMBER(ldlm_request, lock_count);
        CHECK_MEMBER(ldlm_request, lock_desc);
-       CHECK_MEMBER(ldlm_request, lock_handle);
+       CHECK_MEMBER_IS_FLEXIBLE(ldlm_request, lock_handle);
 }
 
 static void

diff --git a/lustre/utils/wiretest.c b/lustre/utils/wiretest.c
index 42c4154..68d686d 100644 (file)
--- a/lustre/utils/wiretest.c
+++ b/lustre/utils/wiretest.c
@@ -3872,7 +3872,7 @@ void lustre_assert_wire_constants(void)
                 (long long)(int)sizeof(((struct ldlm_lock_desc *)0)->l_policy_data));
 
        /* Checks for struct ldlm_request */
-       LASSERTF((int)sizeof(struct ldlm_request) == 104, "found %lld\n",
+       LASSERTF((int)sizeof(struct ldlm_request) == 88, "found %lld\n",
                 (long long)(int)sizeof(struct ldlm_request));
        LASSERTF((int)offsetof(struct ldlm_request, lock_flags) == 0, "found %lld\n",
                 (long long)(int)offsetof(struct ldlm_request, lock_flags));
@@ -3888,8 +3888,9 @@ void lustre_assert_wire_constants(void)
                 (long long)(int)sizeof(((struct ldlm_request *)0)->lock_desc));
        LASSERTF((int)offsetof(struct ldlm_request, lock_handle) == 88, "found %lld\n",
                 (long long)(int)offsetof(struct ldlm_request, lock_handle));
-       LASSERTF((int)sizeof(((struct ldlm_request *)0)->lock_handle) == 16, "found %lld\n",
-                (long long)(int)sizeof(((struct ldlm_request *)0)->lock_handle));
+       LASSERTF((int)sizeof(*((struct ldlm_request *)0)->lock_handle) == 8, "found %lld\n",
+                (long long)(int)sizeof(*((struct ldlm_request *)0)->lock_handle));
+       BUILD_BUG_ON(offsetof(struct ldlm_request, lock_handle) != sizeof(struct ldlm_request));
 
        /* Checks for struct ldlm_reply */
        LASSERTF((int)sizeof(struct ldlm_reply) == 112, "found %lld\n",