[PATCH] btreplay: fix device IO remap functionality

[PATCH] btreplay: fix device IO remap functionality

[Ignat Korchagin]

Commit dd093eb1c48e ("Fix warnings on newer gcc") moved string buffers holding
device names during map file parse stage to stack. However, only pointers to
them are being stored in the allocated "struct map_dev" structure. These
pointers are invalid outside of scope of this function and in a different
thread context. Also "release_map_devs" function still tries to "free" them
later as if they were allocated on the heap.

Moving the buffers back to the heap by instructing "fscanf" to allocate them
while parsing the file.

Alternatively, we could redefine the "struct map_dev" to include the whole
buffers instead of just pointers to them and free them as part of releasing the
whole "struct map_dev".

Fixes: dd093eb1c48e ("Fix warnings on newer gcc")
Signed-off-by: Ignat Korchagin <ignat@cloudflare.com>
---
 btreplay/btreplay.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/btreplay/btreplay.c b/btreplay/btreplay.c
index edaf81f..23cc2a9 100644
--- a/btreplay/btreplay.c
+++ b/btreplay/btreplay.c
@@ -645,7 +645,7 @@ static void find_input_devs(char *idir)
 static void read_map_devs(char *file_name)
 {
  FILE *fp;
- char from_dev[256], to_dev[256];
+ char *from_dev, *to_dev;

  fp = fopen(file_name, "r");
  if (!fp) {
@@ -653,7 +653,7 @@ static void read_map_devs(char *file_name)
  /*NOTREACHED*/
  }

- while (fscanf(fp, "%s %s", from_dev, to_dev) = 2) {
+ while (fscanf(fp, "%ms %ms", &from_dev, &to_dev) = 2) {
  struct map_dev *mdp = malloc(sizeof(*mdp));

  mdp->from_dev = from_dev;
--
2.11.0

Re: [PATCH] btreplay: fix device IO remap functionality

[Jens Axboe]

On 9/16/19 9:39 AM, Ignat Korchagin wrote:
> Commit dd093eb1c48e ("Fix warnings on newer gcc") moved string buffers holding
> device names during map file parse stage to stack. However, only pointers to
> them are being stored in the allocated "struct map_dev" structure. These
> pointers are invalid outside of scope of this function and in a different
> thread context. Also "release_map_devs" function still tries to "free" them
> later as if they were allocated on the heap.
> 
> Moving the buffers back to the heap by instructing "fscanf" to allocate them
> while parsing the file.
> 
> Alternatively, we could redefine the "struct map_dev" to include the whole
> buffers instead of just pointers to them and free them as part of releasing the
> whole "struct map_dev".

Applied - but by hand, your patch is corrupted, it has mangled whitespace.

-- 
Jens Axboe

Re: [PATCH] btreplay: fix device IO remap functionality

[Ignat Korchagin]

Thank you and sorry. I thought Gmail corrupts only tabs, but seems
whitespaces as well. Next time will use git to send it.

On Mon, Sep 16, 2019 at 5:31 PM Jens Axboe <axboe@kernel.dk> wrote:
>
> On 9/16/19 9:39 AM, Ignat Korchagin wrote:
> > Commit dd093eb1c48e ("Fix warnings on newer gcc") moved string buffers holding
> > device names during map file parse stage to stack. However, only pointers to
> > them are being stored in the allocated "struct map_dev" structure. These
> > pointers are invalid outside of scope of this function and in a different
> > thread context. Also "release_map_devs" function still tries to "free" them
> > later as if they were allocated on the heap.
> >
> > Moving the buffers back to the heap by instructing "fscanf" to allocate them
> > while parsing the file.
> >
> > Alternatively, we could redefine the "struct map_dev" to include the whole
> > buffers instead of just pointers to them and free them as part of releasing the
> > whole "struct map_dev".
>
> Applied - but by hand, your patch is corrupted, it has mangled whitespace.
>
> --
> Jens Axboe
>

Re: [PATCH] btreplay: fix device IO remap functionality

[Jens Axboe]

On 9/16/19 11:12 AM, Ignat Korchagin wrote:
> Thank you and sorry. I thought Gmail corrupts only tabs, but seems
> whitespaces as well. Next time will use git to send it.

git send-email is fine with gmail, the web client definitely not...

-- 
Jens Axboe