Theodore Y. Ts'o wrote: > On Mon, May 07, 2018 at 08:16:58PM +0900, Tetsuo Handa wrote: > > Oh, your message did not arrive at news.gmane.org and I didn't notice that you > > already wrote this patch. But please update yours or review mine shown below. > > > > > @@ -673,14 +703,13 @@ static int loop_change_fd(struct loop_device *lo, struct block_device *bdev, > > > if (!file) > > > goto out; > > > > > > + error = loop_validate_file(file, bdev); > > > + if (error) > > > + goto out_putf; > > > + > > > inode = file->f_mapping->host; > > > old_file = lo->lo_backing_file; > > > > > > - error = -EINVAL; > > > - > > > - if (!S_ISREG(inode->i_mode) && !S_ISBLK(inode->i_mode)) > > > - goto out_putf; > > > - > > > /* size of the new backing store needs to be the same */ > > > if (get_loop_size(lo, file) != get_loop_size(lo, old_file)) > > > goto out_putf; > > > > error == 0 upon "goto out_putf" is wrong. > > I don't understand your concern; where are we going to out_putf when > error == 0? - error = -EINVAL; /* <= You are trying to remove this assignment. */ - - if (!S_ISREG(inode->i_mode) && !S_ISBLK(inode->i_mode)) - goto out_putf; /* size of the new backing store needs to be the same */ if (get_loop_size(lo, file) != get_loop_size(lo, old_file)) goto out_putf; /* <= Hence error == 0 at this point. */ By the way, are you aware that current "/* Avoid recursion */" loop is not thread safe?
