queue-5.0/staging-erofs-keep-corrupted-fs-from-crashing-kernel-in-erofs_namei.patch

   1 From 419d6efc50e94bcf5d6b35cd8c71f79edadec564 Mon Sep 17 00:00:00 2001
   2 From: Gao Xiang <gaoxiang25@huawei.com>
   3 Date: Fri, 1 Feb 2019 20:16:31 +0800
   4 Subject: staging: erofs: keep corrupted fs from crashing kernel in erofs_namei()
   5
   6 From: Gao Xiang <gaoxiang25@huawei.com>
   7
   8 commit 419d6efc50e94bcf5d6b35cd8c71f79edadec564 upstream.
   9
  10 As Al pointed out, "
  11 ... and while we are at it, what happens to
  12         unsigned int nameoff = le16_to_cpu(de[mid].nameoff);
  13         unsigned int matched = min(startprfx, endprfx);
  14
  15         struct qstr dname = QSTR_INIT(data + nameoff,
  16                 unlikely(mid >= ndirents - 1) ?
  17                         maxsize - nameoff :
  18                         le16_to_cpu(de[mid + 1].nameoff) - nameoff);
  19
  20         /* string comparison without already matched prefix */
  21         int ret = dirnamecmp(name, &dname, &matched);
  22 if le16_to_cpu(de[...].nameoff) is not monotonically increasing?  I.e.
  23 what's to prevent e.g. (unsigned)-1 ending up in dname.len?
  24
  25 Corrupted fs image shouldn't oops the kernel.. "
  26
  27 Revisit the related lookup flow to address the issue.
  28
  29 Fixes: d72d1ce60174 ("staging: erofs: add namei functions")
  30 Cc: <stable@vger.kernel.org> # 4.19+
  31 Suggested-by: Al Viro <viro@ZenIV.linux.org.uk>
  32 Signed-off-by: Gao Xiang <gaoxiang25@huawei.com>
  33 Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
  34
  35
  36 ---
  37  drivers/staging/erofs/namei.c |  183 ++++++++++++++++++++++--------------------
  38  1 file changed, 97 insertions(+), 86 deletions(-)
  39
  40 --- a/drivers/staging/erofs/namei.c
  41 +++ b/drivers/staging/erofs/namei.c
  42 @@ -15,74 +15,77 @@
  43
  44  #include <trace/events/erofs.h>
  45
  46 -/* based on the value of qn->len is accurate */
  47 -static inline int dirnamecmp(struct qstr *qn,
  48 -       struct qstr *qd, unsigned int *matched)
  49 +struct erofs_qstr {
  50 +       const unsigned char *name;
  51 +       const unsigned char *end;
  52 +};
  53 +
  54 +/* based on the end of qn is accurate and it must have the trailing '\0' */
  55 +static inline int dirnamecmp(const struct erofs_qstr *qn,
  56 +                            const struct erofs_qstr *qd,
  57 +                            unsigned int *matched)
  58  {
  59 -       unsigned int i = *matched, len = min(qn->len, qd->len);
  60 -loop:
  61 -       if (unlikely(i >= len)) {
  62 -               *matched = i;
  63 -               if (qn->len < qd->len) {
  64 -                       /*
  65 -                        * actually (qn->len == qd->len)
  66 -                        * when qd->name[i] == '\0'
  67 -                        */
  68 -                       return qd->name[i] == '\0' ? 0 : -1;
  69 -               }
  70 -               return (qn->len > qd->len);
  71 -       }
  72 +       unsigned int i = *matched;
  73
  74 -       if (qn->name[i] != qd->name[i]) {
  75 -               *matched = i;
  76 -               return qn->name[i] > qd->name[i] ? 1 : -1;
  77 +       /*
  78 +        * on-disk error, let's only BUG_ON in the debugging mode.
  79 +        * otherwise, it will return 1 to just skip the invalid name
  80 +        * and go on (in consideration of the lookup performance).
  81 +        */
  82 +       DBG_BUGON(qd->name > qd->end);
  83 +
  84 +       /* qd could not have trailing '\0' */
  85 +       /* However it is absolutely safe if < qd->end */
  86 +       while (qd->name + i < qd->end && qd->name[i] != '\0') {
  87 +               if (qn->name[i] != qd->name[i]) {
  88 +                       *matched = i;
  89 +                       return qn->name[i] > qd->name[i] ? 1 : -1;
  90 +               }
  91 +               ++i;
  92         }
  93 -
  94 -       ++i;
  95 -       goto loop;
  96 +       *matched = i;
  97 +       /* See comments in __d_alloc on the terminating NUL character */
  98 +       return qn->name[i] == '\0' ? 0 : 1;
  99  }
 100
 101 -static struct erofs_dirent *find_target_dirent(
 102 -       struct qstr *name,
 103 -       u8 *data, int maxsize)
 104 +#define nameoff_from_disk(off, sz)     (le16_to_cpu(off) & ((sz) - 1))
 105 +
 106 +static struct erofs_dirent *find_target_dirent(struct erofs_qstr *name,
 107 +                                              u8 *data,
 108 +                                              unsigned int dirblksize,
 109 +                                              const int ndirents)
 110  {
 111 -       unsigned int ndirents, head, back;
 112 +       int head, back;
 113         unsigned int startprfx, endprfx;
 114         struct erofs_dirent *const de = (struct erofs_dirent *)data;
 115
 116 -       /* make sure that maxsize is valid */
 117 -       BUG_ON(maxsize < sizeof(struct erofs_dirent));
 118 -
 119 -       ndirents = le16_to_cpu(de->nameoff) / sizeof(*de);
 120 -
 121 -       /* corrupted dir (may be unnecessary...) */
 122 -       BUG_ON(!ndirents);
 123 -
 124 -       head = 0;
 125 +       /* since the 1st dirent has been evaluated previously */
 126 +       head = 1;
 127         back = ndirents - 1;
 128         startprfx = endprfx = 0;
 129
 130         while (head <= back) {
 131 -               unsigned int mid = head + (back - head) / 2;
 132 -               unsigned int nameoff = le16_to_cpu(de[mid].nameoff);
 133 +               const int mid = head + (back - head) / 2;
 134 +               const int nameoff = nameoff_from_disk(de[mid].nameoff,
 135 +                                                     dirblksize);
 136                 unsigned int matched = min(startprfx, endprfx);
 137 -
 138 -               struct qstr dname = QSTR_INIT(data + nameoff,
 139 -                       unlikely(mid >= ndirents - 1) ?
 140 -                               maxsize - nameoff :
 141 -                               le16_to_cpu(de[mid + 1].nameoff) - nameoff);
 142 +               struct erofs_qstr dname = {
 143 +                       .name = data + nameoff,
 144 +                       .end = unlikely(mid >= ndirents - 1) ?
 145 +                               data + dirblksize :
 146 +                               data + nameoff_from_disk(de[mid + 1].nameoff,
 147 +                                                        dirblksize)
 148 +               };
 149
 150                 /* string comparison without already matched prefix */
 151                 int ret = dirnamecmp(name, &dname, &matched);
 152
 153 -               if (unlikely(!ret))
 154 +               if (unlikely(!ret)) {
 155                         return de + mid;
 156 -               else if (ret > 0) {
 157 +               } else if (ret > 0) {
 158                         head = mid + 1;
 159                         startprfx = matched;
 160 -               } else if (unlikely(mid < 1))   /* fix "mid" overflow */
 161 -                       break;
 162 -               else {
 163 +               } else {
 164                         back = mid - 1;
 165                         endprfx = matched;
 166                 }
 167 @@ -91,12 +94,12 @@ static struct erofs_dirent *find_target_
 168         return ERR_PTR(-ENOENT);
 169  }
 170
 171 -static struct page *find_target_block_classic(
 172 -       struct inode *dir,
 173 -       struct qstr *name, int *_diff)
 174 +static struct page *find_target_block_classic(struct inode *dir,
 175 +                                             struct erofs_qstr *name,
 176 +                                             int *_ndirents)
 177  {
 178         unsigned int startprfx, endprfx;
 179 -       unsigned int head, back;
 180 +       int head, back;
 181         struct address_space *const mapping = dir->i_mapping;
 182         struct page *candidate = ERR_PTR(-ENOENT);
 183
 184 @@ -105,41 +108,43 @@ static struct page *find_target_block_cl
 185         back = inode_datablocks(dir) - 1;
 186
 187         while (head <= back) {
 188 -               unsigned int mid = head + (back - head) / 2;
 189 +               const int mid = head + (back - head) / 2;
 190                 struct page *page = read_mapping_page(mapping, mid, NULL);
 191
 192 -               if (IS_ERR(page)) {
 193 -exact_out:
 194 -                       if (!IS_ERR(candidate)) /* valid candidate */
 195 -                               put_page(candidate);
 196 -                       return page;
 197 -               } else {
 198 -                       int diff;
 199 -                       unsigned int ndirents, matched;
 200 -                       struct qstr dname;
 201 +               if (!IS_ERR(page)) {
 202                         struct erofs_dirent *de = kmap_atomic(page);
 203 -                       unsigned int nameoff = le16_to_cpu(de->nameoff);
 204 -
 205 -                       ndirents = nameoff / sizeof(*de);
 206 +                       const int nameoff = nameoff_from_disk(de->nameoff,
 207 +                                                             EROFS_BLKSIZ);
 208 +                       const int ndirents = nameoff / sizeof(*de);
 209 +                       int diff;
 210 +                       unsigned int matched;
 211 +                       struct erofs_qstr dname;
 212
 213 -                       /* corrupted dir (should have one entry at least) */
 214 -                       BUG_ON(!ndirents || nameoff > PAGE_SIZE);
 215 +                       if (unlikely(!ndirents)) {
 216 +                               DBG_BUGON(1);
 217 +                               kunmap_atomic(de);
 218 +                               put_page(page);
 219 +                               page = ERR_PTR(-EIO);
 220 +                               goto out;
 221 +                       }
 222
 223                         matched = min(startprfx, endprfx);
 224
 225                         dname.name = (u8 *)de + nameoff;
 226 -                       dname.len = ndirents == 1 ?
 227 -                               /* since the rest of the last page is 0 */
 228 -                               EROFS_BLKSIZ - nameoff
 229 -                               : le16_to_cpu(de[1].nameoff) - nameoff;
 230 +                       if (ndirents == 1)
 231 +                               dname.end = (u8 *)de + EROFS_BLKSIZ;
 232 +                       else
 233 +                               dname.end = (u8 *)de +
 234 +                                       nameoff_from_disk(de[1].nameoff,
 235 +                                                         EROFS_BLKSIZ);
 236
 237                         /* string comparison without already matched prefix */
 238                         diff = dirnamecmp(name, &dname, &matched);
 239                         kunmap_atomic(de);
 240
 241                         if (unlikely(!diff)) {
 242 -                               *_diff = 0;
 243 -                               goto exact_out;
 244 +                               *_ndirents = 0;
 245 +                               goto out;
 246                         } else if (diff > 0) {
 247                                 head = mid + 1;
 248                                 startprfx = matched;
 249 @@ -147,45 +152,51 @@ exact_out:
 250                                 if (likely(!IS_ERR(candidate)))
 251                                         put_page(candidate);
 252                                 candidate = page;
 253 +                               *_ndirents = ndirents;
 254                         } else {
 255                                 put_page(page);
 256
 257 -                               if (unlikely(mid < 1))  /* fix "mid" overflow */
 258 -                                       break;
 259 -
 260                                 back = mid - 1;
 261                                 endprfx = matched;
 262                         }
 263 +                       continue;
 264                 }
 265 +out:           /* free if the candidate is valid */
 266 +               if (!IS_ERR(candidate))
 267 +                       put_page(candidate);
 268 +               return page;
 269         }
 270 -       *_diff = 1;
 271         return candidate;
 272  }
 273
 274  int erofs_namei(struct inode *dir,
 275 -       struct qstr *name,
 276 -       erofs_nid_t *nid, unsigned int *d_type)
 277 +               struct qstr *name,
 278 +               erofs_nid_t *nid, unsigned int *d_type)
 279  {
 280 -       int diff;
 281 +       int ndirents;
 282         struct page *page;
 283 -       u8 *data;
 284 +       void *data;
 285         struct erofs_dirent *de;
 286 +       struct erofs_qstr qn;
 287
 288         if (unlikely(!dir->i_size))
 289                 return -ENOENT;
 290
 291 -       diff = 1;
 292 -       page = find_target_block_classic(dir, name, &diff);
 293 +       qn.name = name->name;
 294 +       qn.end = name->name + name->len;
 295 +
 296 +       ndirents = 0;
 297 +       page = find_target_block_classic(dir, &qn, &ndirents);
 298
 299         if (unlikely(IS_ERR(page)))
 300                 return PTR_ERR(page);
 301
 302         data = kmap_atomic(page);
 303         /* the target page has been mapped */
 304 -       de = likely(diff) ?
 305 -               /* since the rest of the last page is 0 */
 306 -               find_target_dirent(name, data, EROFS_BLKSIZ) :
 307 -               (struct erofs_dirent *)data;
 308 +       if (ndirents)
 309 +               de = find_target_dirent(&qn, data, EROFS_BLKSIZ, ndirents);
 310 +       else
 311 +               de = (struct erofs_dirent *)data;
 312
 313         if (likely(!IS_ERR(de))) {
 314                 *nid = le64_to_cpu(de->nid);