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.

Lustre-change: https://review.whamcloud.com/54926
Lustre-commit: TBD (from 765bf07e894178a6a6f1477559a793af3a52412e)

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/ex/lustre-release/+/54941
Reviewed-by: Jian Yu <yujian@whamcloud.com>
Tested-by: jenkins <devops@whamcloud.com>

diff --git a/lustre/include/uapi/linux/lustre/lustre_idl.h b/lustre/include/uapi/linux/lustre/lustre_idl.h
index 9118125..3aa7ae7 100644 (file)
--- a/lustre/include/uapi/linux/lustre/lustre_idl.h
+++ b/lustre/include/uapi/linux/lustre/lustre_idl.h
@@ -2638,10 +2638,10 @@ struct ldlm_lock_desc {
 #define LDLM_ENQUEUE_CANCEL_OFF 1
 
 struct ldlm_request {
-       __u32 lock_flags;               /* LDLM_FL_*, see lustre_dlm_flags.h */
-       __u32 lock_count;               /* number of locks in lock_handle[] */
-       struct ldlm_lock_desc lock_desc;/* lock descriptor */
-       struct lustre_handle lock_handle[LDLM_LOCKREQ_HANDLES];
+       __u32 lock_flags;                /* LDLM_FL_*, see lustre_dlm_flags.h */
+       __u32 lock_count;                /* number of locks in lock_handle[] */
+       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 18d7eec..5ae1b8f 100644 (file)
--- a/lustre/ldlm/ldlm_lockd.c
+++ b/lustre/ldlm/ldlm_lockd.c
@@ -1742,9 +1742,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 301b5ce..09ef72b 100644 (file)
--- a/lustre/ldlm/ldlm_request.c
+++ b/lustre/ldlm/ldlm_request.c
@@ -89,24 +89,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 2d5921d..2af3a3e 100644 (file)
--- a/lustre/ptlrpc/layout.c
+++ b/lustre/ptlrpc/layout.c
@@ -1115,9 +1115,10 @@ struct req_msg_field RMF_CONNECT_DATA =
 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),
-                    lustre_swab_ldlm_request, NULL);
+       DEFINE_MSGF("dlm_req", RMF_F_NO_SIZE_CHECK /* ldlm_request_bufsize */,
+                   offsetof(struct ldlm_request,
+                            lock_handle[LDLM_LOCKREQ_HANDLES]),
+                   lustre_swab_ldlm_request, NULL);
 EXPORT_SYMBOL(RMF_DLM_REQ);
 
 struct req_msg_field RMF_DLM_REP =

diff --git a/lustre/ptlrpc/wiretest.c b/lustre/ptlrpc/wiretest.c
index 489b3a1..046e83d 100644 (file)
--- a/lustre/ptlrpc/wiretest.c
+++ b/lustre/ptlrpc/wiretest.c
@@ -3853,7 +3853,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));
@@ -3869,8 +3869,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 e2bf937..f53fec8 100644 (file)
--- a/lustre/utils/wirecheck.c
+++ b/lustre/utils/wirecheck.c
@@ -1718,7 +1718,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 254976d..7a5d39e 100644 (file)
--- a/lustre/utils/wiretest.c
+++ b/lustre/utils/wiretest.c
@@ -3888,7 +3888,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));
@@ -3904,8 +3904,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",