LU-12811 ptlrpc: pass buffer size to the swabbing functions

By adding a separate rmf_swab_len() function pointer to
req_msg_field, the buffer size can be passed to the swabbing
functions, e.g. lustre_swab_fiemap() in this patch, to avoid
invalid access, especially for small buffer.

Signed-off-by: Emoly Liu <emoly@whamcloud.com>
Change-Id: I997e6a828f2f1cdfdb8a5fa241fa43ca0ae3677e
Reviewed-on: https://review.whamcloud.com/36435
Tested-by: jenkins <devops@whamcloud.com>
Reviewed-by: Andreas Dilger <adilger@whamcloud.com>
Tested-by: Maloo <maloo@whamcloud.com>
Reviewed-by: Mike Pershin <mpershin@whamcloud.com>

diff --git a/lustre/include/lustre_swab.h b/lustre/include/lustre_swab.h
index 96dcd49..720f7a2 100644 (file)
--- a/lustre/include/lustre_swab.h
+++ b/lustre/include/lustre_swab.h
@@ -92,7 +92,7 @@ void lustre_swab_lfsck_reply(struct lfsck_reply *lr);
 void lustre_swab_obdo(struct obdo *o);
 void lustre_swab_ost_body(struct ost_body *b);
 void lustre_swab_ost_last_id(__u64 *id);
 void lustre_swab_obdo(struct obdo *o);
 void lustre_swab_ost_body(struct ost_body *b);
 void lustre_swab_ost_last_id(__u64 *id);

-void lustre_swab_fiemap(struct fiemap *fiemap);
+int lustre_swab_fiemap(struct fiemap *fiemap, __u32 len);

 void lustre_swab_fiemap_info_key(struct ll_fiemap_info_key *fiemap_info);
 void lustre_swab_lov_user_md_v1(struct lov_user_md_v1 *lum);
 void lustre_swab_lov_user_md_v3(struct lov_user_md_v3 *lum);
 void lustre_swab_fiemap_info_key(struct ll_fiemap_info_key *fiemap_info);
 void lustre_swab_lov_user_md_v1(struct lov_user_md_v1 *lum);
 void lustre_swab_lov_user_md_v3(struct lov_user_md_v3 *lum);

diff --git a/lustre/ptlrpc/layout.c b/lustre/ptlrpc/layout.c
index 3fb3099..895ace3 100644 (file)
--- a/lustre/ptlrpc/layout.c
+++ b/lustre/ptlrpc/layout.c
@@ -848,17 +848,23 @@ static struct req_format *req_formats[] = {
 };
 
 struct req_msg_field {
 };
 
 struct req_msg_field {

-        const __u32 rmf_flags;
-        const char  *rmf_name;
-        /**
-         * Field length. (-1) means "variable length".  If the
-         * \a RMF_F_STRUCT_ARRAY flag is set the field is also variable-length,
-         * but the actual size must be a whole multiple of \a rmf_size.
-         */
-        const int   rmf_size;
-        void        (*rmf_swabber)(void *);
-        void        (*rmf_dumper)(void *);
-        int         rmf_offset[ARRAY_SIZE(req_formats)][RCL_NR];
+       const __u32 rmf_flags;
+       const char  *rmf_name;
+       /**
+        * Field length. (-1) means "variable length".  If the
+        * \a RMF_F_STRUCT_ARRAY flag is set the field is also variable-length,
+        * but the actual size must be a whole multiple of \a rmf_size.
+        */
+       const int   rmf_size;
+       void        (*rmf_swabber)(void *);
+       /**
+        * Pass buffer size to swabbing function
+        * \retval      > 0             the number of bytes swabbed
+        *              -EOVERFLOW      on error
+        */
+       int         (*rmf_swab_len)(void *, __u32);
+       void        (*rmf_dumper)(void *);
+       int         rmf_offset[ARRAY_SIZE(req_formats)][RCL_NR];

 };
 
 enum rmf_flags {
 };
 
 enum rmf_flags {


@@ -891,6 +897,14 @@ struct req_capsule;
         .rmf_dumper  = (void (*)(void*))(dumper)                \
 }
 
         .rmf_dumper  = (void (*)(void*))(dumper)                \
 }
 

+#define DEFINE_MSGFL(name, flags, size, swab_len, dumper) {    \
+       .rmf_name     = (name),                                 \
+       .rmf_flags    = (flags),                                \
+       .rmf_size     = (size),                                 \
+       .rmf_swab_len = (int (*)(void *, __u32))(swab_len),     \
+       .rmf_dumper   = (void (*)(void *))(dumper)              \
+}
+

 struct req_msg_field RMF_GENERIC_DATA =
         DEFINE_MSGF("generic_data", 0,
                     -1, NULL, NULL);
 struct req_msg_field RMF_GENERIC_DATA =
         DEFINE_MSGF("generic_data", 0,
                     -1, NULL, NULL);


@@ -1203,7 +1217,7 @@ struct req_msg_field RMF_FIEMAP_KEY =
 EXPORT_SYMBOL(RMF_FIEMAP_KEY);
 
 struct req_msg_field RMF_FIEMAP_VAL =
 EXPORT_SYMBOL(RMF_FIEMAP_KEY);
 
 struct req_msg_field RMF_FIEMAP_VAL =

-        DEFINE_MSGF("fiemap", 0, -1, lustre_swab_fiemap, NULL);
+       DEFINE_MSGFL("fiemap", 0, -1, lustre_swab_fiemap, NULL);

 EXPORT_SYMBOL(RMF_FIEMAP_VAL);
 
 struct req_msg_field RMF_IDX_INFO =
 EXPORT_SYMBOL(RMF_FIEMAP_VAL);
 
 struct req_msg_field RMF_IDX_INFO =


@@ -1978,27 +1992,28 @@ __u32 __req_capsule_offset(const struct req_capsule *pill,
  * Helper for __req_capsule_get(); swabs value / array of values and/or dumps
  * them if desired.
  */
  * Helper for __req_capsule_get(); swabs value / array of values and/or dumps
  * them if desired.
  */

-static
-void
+static int

 swabber_dumper_helper(struct req_capsule *pill,
 swabber_dumper_helper(struct req_capsule *pill,

-                      const struct req_msg_field *field,
-                      enum req_location loc,
-                      int offset,
-                      void *value, int len, int dump, void (*swabber)( void *))
+                     const struct req_msg_field *field,
+                     enum req_location loc,
+                     int offset,
+                     void *value, int len, int dump, void (*swabber)(void *))

 {
 {

-        void    *p;
-        int     i;
-        int     n;
-        int     do_swab;
-        int     inout = loc == RCL_CLIENT;
-
-        swabber = swabber ?: field->rmf_swabber;
-
-        if (ptlrpc_buf_need_swab(pill->rc_req, inout, offset) &&
-            swabber != NULL && value != NULL)
-                do_swab = 1;
-        else
-                do_swab = 0;
+       void    *p;
+       int     i;
+       int     n;
+       int     size;
+       int     rc = 0;
+       int     do_swab;
+       int     inout = loc == RCL_CLIENT;
+
+       swabber = swabber ?: field->rmf_swabber;
+
+       if (ptlrpc_buf_need_swab(pill->rc_req, inout, offset) &&
+           (swabber != NULL || field->rmf_swab_len != NULL) && value != NULL)
+               do_swab = 1;
+       else
+               do_swab = 0;

 
        if (!field->rmf_dumper)
                dump = 0;
 
        if (!field->rmf_dumper)
                dump = 0;


@@ -2009,24 +2024,38 @@ swabber_dumper_helper(struct req_capsule *pill,
                                do_swab ? "unswabbed " : "", field->rmf_name);
                         field->rmf_dumper(value);
                 }
                                do_swab ? "unswabbed " : "", field->rmf_name);
                         field->rmf_dumper(value);
                 }

-                if (!do_swab)
-                        return;
-                swabber(value);
-                ptlrpc_buf_set_swabbed(pill->rc_req, inout, offset);
+               if (!do_swab)
+                       return 0;
+               if (!field->rmf_swab_len) {
+                       swabber(value);
+               } else {
+                       size = field->rmf_swab_len(value, len);
+                       if (size < 0)
+                               rc = size;
+               }
+               ptlrpc_buf_set_swabbed(pill->rc_req, inout, offset);

                if (dump) {
                         CDEBUG(D_RPCTRACE, "Dump of swabbed field %s "
                                "follows\n", field->rmf_name);
                         field->rmf_dumper(value);
                 }
 
                if (dump) {
                         CDEBUG(D_RPCTRACE, "Dump of swabbed field %s "
                                "follows\n", field->rmf_name);
                         field->rmf_dumper(value);
                 }
 

-                return;
-        }
+               return rc;
+       }

 
 

-        /*
-         * We're swabbing an array; swabber() swabs a single array element, so
-         * swab every element.
-         */
-        LASSERT((len % field->rmf_size) == 0);
+       /*
+        * We're swabbing an array; swabber() swabs a single array element, so
+        * swab every element.
+        */
+       if (len % field->rmf_size) {
+               static const struct req_msg_field *last_field;
+
+               if (field != last_field) {
+                       CERROR("%s: array buffer size %u is not a multiple of element size %u\n",
+                              field->rmf_name, len, field->rmf_size);
+                       last_field = field;
+               }
+       }

         for (p = value, i = 0, n = len / field->rmf_size;
              i < n;
              i++, p += field->rmf_size) {
         for (p = value, i = 0, n = len / field->rmf_size;
              i < n;
              i++, p += field->rmf_size) {


@@ -2038,7 +2067,17 @@ swabber_dumper_helper(struct req_capsule *pill,
                 }
                 if (!do_swab)
                         continue;
                 }
                 if (!do_swab)
                         continue;

-                swabber(p);
+               if (!field->rmf_swab_len) {
+                       swabber(p);
+               } else {
+                       size = field->rmf_swab_len(p, len);
+                       if (size > 0) {
+                               len -= size;
+                       } else {
+                               rc = size;
+                               break;
+                       }
+               }

                 if (dump) {
                         CDEBUG(D_RPCTRACE, "Dump of swabbed array field %s, "
                                "element %d follows\n", field->rmf_name, i);
                 if (dump) {
                         CDEBUG(D_RPCTRACE, "Dump of swabbed array field %s, "
                                "element %d follows\n", field->rmf_name, i);


@@ -2047,6 +2086,8 @@ swabber_dumper_helper(struct req_capsule *pill,
         }
         if (do_swab)
                 ptlrpc_buf_set_swabbed(pill->rc_req, inout, offset);
         }
         if (do_swab)
                 ptlrpc_buf_set_swabbed(pill->rc_req, inout, offset);

+
+       return rc;

 }
 
 /**
 }
 
 /**

diff --git a/lustre/ptlrpc/pack_generic.c b/lustre/ptlrpc/pack_generic.c
index 0f138ae..91639eb 100644 (file)
--- a/lustre/ptlrpc/pack_generic.c
+++ b/lustre/ptlrpc/pack_generic.c
@@ -2098,14 +2098,25 @@ static void lustre_swab_fiemap_hdr(struct fiemap *fiemap)
        __swab32s(&fiemap->fm_reserved);
 }
 
        __swab32s(&fiemap->fm_reserved);
 }
 

-void lustre_swab_fiemap(struct fiemap *fiemap)
+int lustre_swab_fiemap(struct fiemap *fiemap, __u32 len)

 {
 {

-       __u32 i;
+       __u32 i, size, count;

 
        lustre_swab_fiemap_hdr(fiemap);
 
 
        lustre_swab_fiemap_hdr(fiemap);
 

-       for (i = 0; i < fiemap->fm_mapped_extents; i++)
+       size = fiemap_count_to_size(fiemap->fm_mapped_extents);
+       count = fiemap->fm_mapped_extents;
+       if (unlikely(size > len)) {
+               count = (len - sizeof(struct fiemap)) /
+                       sizeof(struct fiemap_extent);
+               fiemap->fm_mapped_extents = count;
+               size = -EOVERFLOW;
+       }
+       /* still swab extents as we cannot yet pass rc to callers */
+       for (i = 0; i < count; i++)

                lustre_swab_fiemap_extent(&fiemap->fm_extents[i]);
                lustre_swab_fiemap_extent(&fiemap->fm_extents[i]);

+
+       return size;

 }
 
 void lustre_swab_fiemap_info_key(struct ll_fiemap_info_key *fiemap_info)
 }
 
 void lustre_swab_fiemap_info_key(struct ll_fiemap_info_key *fiemap_info)