Whamcloud - gitweb
LU-11798 grant: prevent overflow of o_undirty 48/33948/10
authorAlex Zhuravlev <bzzz@whamcloud.com>
Wed, 2 Jan 2019 18:11:29 +0000 (10:11 -0800)
committerOleg Drokin <green@whamcloud.com>
Mon, 1 Apr 2019 07:22:07 +0000 (07:22 +0000)
tgt_grant_inflate() returns a u64, and if tgd_blockbits and val are
large enough, can return a value >= 2^32.  tgt_grant_incoming()
assigns oa->o_undirty the returned value.  Since o_undirty is u32, it
can overflow.

This occurs with Lustre clients < 2.10 and a ZFS backend when the zfs
"recordsize" > 128k (the default).

In tgt_grant_inflate(), check the returned value and prevent o_undirty
from being assigned a value greater than 2^30.

Change-Id: I75b9065a524238df2d582e640418fdfa2f1e9a72
Signed-off-by: Alexey Zhuravlev <bzzz@whamcloud.com>
Signed-off-by: Olaf Faaland <faaland1@llnl.gov>
Reviewed-on: https://review.whamcloud.com/33948
Reviewed-by: Andreas Dilger <adilger@whamcloud.com>
Tested-by: Jenkins
Tested-by: Maloo <maloo@whamcloud.com>
Reviewed-by: Patrick Farrell <pfarrell@whamcloud.com>
Reviewed-by: Oleg Drokin <green@whamcloud.com>
lustre/osc/osc_request.c
lustre/target/tgt_grant.c

index f3a9f38..a8c0e5c 100644 (file)
@@ -692,8 +692,8 @@ static void osc_announce_cached(struct client_obd *cli, struct obdo *oa,
                /* Do not ask for more than OBD_MAX_GRANT - a margin for server
                 * to add extent tax, etc.
                 */
-               oa->o_undirty = min(undirty, OBD_MAX_GRANT -
-                                   (PTLRPC_MAX_BRW_PAGES << PAGE_SHIFT)*4UL);
+               oa->o_undirty = min(undirty, OBD_MAX_GRANT &
+                                   ~(PTLRPC_MAX_BRW_SIZE * 4UL));
         }
        oa->o_grant = cli->cl_avail_grant + cli->cl_reserved_grant;
         oa->o_dropped = cli->cl_lost_grant;
index 386fa60..f6ee8d1 100644 (file)
@@ -499,8 +499,7 @@ static void tgt_grant_incoming(const struct lu_env *env, struct obd_export *exp,
        struct tg_export_data   *ted = &exp->exp_target_data;
        struct obd_device       *obd = exp->exp_obd;
        struct tg_grants_data   *tgd = &obd->u.obt.obt_lut->lut_tgd;
-       long                     dirty;
-       long                     dropped;
+       long long                dirty, dropped;
        ENTRY;
 
        assert_spin_locked(&tgd->tgd_grant_lock);
@@ -524,10 +523,19 @@ static void tgt_grant_incoming(const struct lu_env *env, struct obd_export *exp,
 
        /* inflate grant counters if required */
        if (!exp_grant_param_supp(exp)) {
+               u64 tmp;
                oa->o_grant     = tgt_grant_inflate(tgd, oa->o_grant);
                oa->o_dirty     = tgt_grant_inflate(tgd, oa->o_dirty);
-               oa->o_dropped   = tgt_grant_inflate(tgd, (u64)oa->o_dropped);
-               oa->o_undirty   = tgt_grant_inflate(tgd, oa->o_undirty);
+               /* inflation can bump client's wish to >4GB which doesn't fit
+                * 32bit o_undirty, limit that ..  */
+               tmp = tgt_grant_inflate(tgd, oa->o_undirty);
+               if (tmp >= OBD_MAX_GRANT)
+                       tmp = OBD_MAX_GRANT & ~(1ULL << tgd->tgd_blockbits);
+               oa->o_undirty = tmp;
+               tmp = tgt_grant_inflate(tgd, oa->o_dropped);
+               if (tmp >= OBD_MAX_GRANT)
+                       tmp = OBD_MAX_GRANT & ~(1ULL << tgd->tgd_blockbits);
+               oa->o_dropped = tmp;
        }
 
        dirty = oa->o_dirty;
@@ -542,13 +550,13 @@ static void tgt_grant_incoming(const struct lu_env *env, struct obd_export *exp,
        tgd->tgd_tot_dirty += dirty - ted->ted_dirty;
        if (ted->ted_grant < dropped) {
                CDEBUG(D_CACHE,
-                      "%s: cli %s/%p reports %lu dropped > grant %lu\n",
+                      "%s: cli %s/%p reports %llu dropped > grant %lu\n",
                       obd->obd_name, exp->exp_client_uuid.uuid, exp, dropped,
                       ted->ted_grant);
                dropped = 0;
        }
        if (tgd->tgd_tot_granted < dropped) {
-               CERROR("%s: cli %s/%p reports %lu dropped > tot_grant %llu\n",
+               CERROR("%s: cli %s/%p reports %llu dropped > tot_grant %llu\n",
                       obd->obd_name, exp->exp_client_uuid.uuid, exp,
                       dropped, tgd->tgd_tot_granted);
                dropped = 0;