xfs
[Top] [All Lists]

[PATCH V2] xfsprogs: Fix possible unallocated memory access in fiemap

To: Dave Chinner <david@xxxxxxxxxxxxx>
Subject: [PATCH V2] xfsprogs: Fix possible unallocated memory access in fiemap
From: Eric Sandeen <sandeen@xxxxxxxxxxx>
Date: Fri, 25 Jan 2013 15:10:22 -0600
Cc: LukÃÅ Czerner <lczerner@xxxxxxxxxx>, hch@xxxxxxxxxxxxx, xfs@xxxxxxxxxxx
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <20121213230608.GI16353@dastard>
References: <1355319059-12111-1-git-send-email-lczerner@xxxxxxxxxx> <20121212231245.GY16353@dastard> <alpine.LFD.2.00.1212131024460.17970@localhost> <20121213230608.GI16353@dastard>
User-agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.8; rv:17.0) Gecko/20130107 Thunderbird/17.0.2
(Based on original patch by Lukas Czerner & comments by Dave Chinner)

Currently we could access unallocated memory in fiemap because we're
using uninitialized variable 'fiemap' in fiemap_f(). In fact this has
been spotted on x390s machine where xfs_io would segfault.

The problem happens in the for cycle which seems to be intended to
compute the header item spacing. However at that point the fiemap
structure has just been allocated and does not contain any extents
yet, so it is entirely useless and it never actually worked.

This patch delays the format calculation until the first batch
of extents has come in for analysis.

Signed-off-by: Eric Sandeen <sandeen@xxxxxxxxxx>
---

diff --git a/io/fiemap.c b/io/fiemap.c
index e32a416..e76be97 100644
--- a/io/fiemap.c
+++ b/io/fiemap.c
@@ -90,6 +90,14 @@ print_verbose(
        memset(lbuf, 0, sizeof(lbuf));
        memset(bbuf, 0, sizeof(bbuf));
 
+       if (*cur_extent == 0) {
+               printf("%4s: %-*s %-*s %*s %*s\n", _("EXT"),
+                       foff_w, _("FILE-OFFSET"),
+                       boff_w, _("BLOCK-RANGE"),
+                       tot_w, _("TOTAL"),
+                       flg_w, _("FLAGS"));
+       }
+
        if (lstart != llast) {
                snprintf(lbuf, sizeof(lbuf), "[%llu..%llu]:", llast,
                         lstart - 1ULL);
@@ -157,6 +165,47 @@ print_plain(
        *last_logical = extent->fe_logical + extent->fe_length;
 }
 
+/*
+ * Calculate the proper extent table format based on first
+ * set of extents
+ */
+static void
+calc_print_format(
+       struct fiemap           *fiemap,
+       __u64                   blocksize,
+       int                     *foff_w,
+       int                     *boff_w,
+       int                     *tot_w,
+       int                     *flg_w)
+{
+       int                     i;
+       char                    lbuf[32];
+       char                    bbuf[32];
+       __u64                   logical;
+       __u64                   block;
+       __u64                   len;
+       struct fiemap_extent    *extent;
+
+       for (i = 0; i < fiemap->fm_mapped_extents; i++) {
+
+               extent = &fiemap->fm_extents[i];
+               logical = extent->fe_logical / blocksize;
+               len = extent->fe_length / blocksize;
+               block = extent->fe_physical / blocksize;
+
+               snprintf(lbuf, sizeof(lbuf), "[%llu..%llu]", logical,
+                        logical + len - 1);
+               snprintf(bbuf, sizeof(bbuf), "%llu..%llu", block,
+                        block + len - 1);
+               *foff_w = max(*foff_w, strlen(lbuf));
+               *boff_w = max(*boff_w, strlen(bbuf));
+               *tot_w = max(*tot_w, numlen(len, 10));
+               *flg_w = max(*flg_w, numlen(extent->fe_flags, 16));
+               if (extent->fe_flags & FIEMAP_EXTENT_LAST)
+                       break;
+       }
+}
+
 int
 fiemap_f(
        int             argc,
@@ -215,38 +264,6 @@ fiemap_f(
 
        printf("%s:\n", file->name);
 
-       if (vflag) {
-               for (i = 0; i < fiemap->fm_mapped_extents; i++) {
-                       char                    lbuf[32];
-                       char                    bbuf[32];
-                       __u64                   logical;
-                       __u64                   block;
-                       __u64                   len;
-                       struct fiemap_extent    *extent;
-
-                       extent = &fiemap->fm_extents[i];
-                       logical = extent->fe_logical / blocksize;
-                       len = extent->fe_length / blocksize;
-                       block = extent->fe_physical / blocksize;
-
-                       snprintf(lbuf, sizeof(lbuf), "[%llu..%llu]", logical,
-                                logical + len - 1);
-                       snprintf(bbuf, sizeof(bbuf), "%llu..%llu", block,
-                                block + len - 1);
-                       foff_w = max(foff_w, strlen(lbuf));
-                       boff_w = max(boff_w, strlen(bbuf));
-                       tot_w = max(tot_w, numlen(len, 10));
-                       flg_w = max(flg_w, numlen(extent->fe_flags, 16));
-                       if (extent->fe_flags & FIEMAP_EXTENT_LAST)
-                               break;
-               }
-               printf("%4s: %-*s %-*s %*s %*s\n", _("EXT"),
-                      foff_w, _("FILE-OFFSET"),
-                      boff_w, _("BLOCK-RANGE"),
-                      tot_w, _("TOTAL"),
-                      flg_w, _("FLAGS"));
-       }
-
        while (!last && ((cur_extent + 1) != max_extents)) {
                if (max_extents)
                        num_extents = min(num_extents,
@@ -275,12 +292,18 @@ fiemap_f(
                        struct fiemap_extent    *extent;
 
                        extent = &fiemap->fm_extents[i];
-                       if (vflag)
+                       if (vflag) {
+                               if (cur_extent == 0) {
+                                       calc_print_format(fiemap, blocksize,
+                                                         &foff_w, &boff_w,
+                                                         &tot_w, &flg_w);
+                               }
+                                       
                                print_verbose(extent, blocksize, foff_w,
                                              boff_w, tot_w, flg_w,
                                              max_extents, &cur_extent,
                                              &last_logical);
-                       else
+                       } else
                                print_plain(extent, lflag, blocksize,
                                            max_extents, &cur_extent,
                                            &last_logical);


<Prev in Thread] Current Thread [Next in Thread>
  • [PATCH V2] xfsprogs: Fix possible unallocated memory access in fiemap, Eric Sandeen <=