On Fri, May 11, 2018 at 08:25:27AM +0200, Christoph Hellwig wrote:
> On Thu, May 10, 2018 at 08:08:38AM -0700, Darrick J. Wong wrote:
> > > > > + sector_t *bno = data;
> > > > > +
> > > > > + if (iomap->type == IOMAP_MAPPED)
> > > > > + *bno = (iomap->addr + pos - iomap->offset) >> inode->i_blkbits;
> > > >
> > > > Does this need to be careful w.r.t. overflow on systems where sector_t
> > > > is a 32-bit unsigned long?
> > > >
> > > > Also, ioctl_fibmap() typecasts the returned sector_t to an int, which
> > > > also seems broken. I agree the interface needs to die, but ioctls take
> > > > a long time to deprecate.
> > >
> > > Not much we can do about the interface.
> >
> > Yes, the interface is fubar, but if file /foo maps to block 8589934720
> > then do we return the truncated result 128?
>
> Then we'll get a corrupt result. What do you think we could do here
> eithere in the old or new code?
I think the only thing we /can/ do is figure out if we'd be truncating
the result, dump a warning to the kernel, and return 0, because we don't
want smartypants FIBMAP callers to be using crap block pointers.
Something like this for the bmap implementation...
uint64_t mapping = iomap->addr;
#ifdef CONFIG_LBDAF
if (mapping > ULONG_MAX) {
/* Do not truncate results. */
return 0;
}
#endif
...and in the bmap ioctl...
sector_t mapping = ...;
if (mapping > INT_MAX) {
WARN(1, "would truncate bmap result, go fix your stupid program");
return 0;
}
--D
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
