ffs: Clamp BIO_SPEEDUP length

[markj]

Summary

On 32-bit platforms, the computed size of the BIO_SPEEDUP submitted by
softdep_request_cleanup() may be negative when assigned to bp->b_bcount,
which has type "long".

Clamp the size to LONG_MAX. We could perhaps limit it to IOSIZE_MAX,
but that would reduce the limit to 2GB on 64-bit platforms, and I'm not
sure if that's desirable.

[kib]

sys/ufs/ffs/ffs_softdep.c

1467 ↗

(On Diff #79147)

Wouldn't better type for shortage be ufs2_daddr_t or even off_t ?

[markj]

sys/ufs/ffs/ffs_softdep.c

1467 ↗

(On Diff #79147)

I thought about using ufs2_daddr_t since that's the type used by the caller, but it seemed kind of strange to me. off_t seems more reasonable.

[chs]

The same issue exists in g_io_speedup() (though that function appears to be unused at the moment).

[imp]

sys/ufs/ffs/ffs_softdep.c

1477 ↗

(On Diff #79147)

don't we have a min variant we can use here?

min(shortage, LONG_MAX)

????

[imp]

I'd have thought that b_bcount would have been a size_t or off_t too like bio_length is, not a long too....
Maybe we should kassert b_bcount >= 0 when we submit to strategy since it is weirdly signed.

[markj]

• Use off_t.
• Use omin().

[markj]

Update g_io_speedup() as well.

[markj]

In D27081#604389, @chs wrote:

The same issue exists in g_io_speedup() (though that function appears to be unused at the moment).

It's not quite the same since there's no sign extension happening, I believe.

[imp]

I wonder if we can hit this issue inside of ffs_blkfree as well, since we might free > 2GB in one shot on i386 as well...

[markj]

In D27081#604410, @imp wrote:

I wonder if we can hit this issue inside of ffs_blkfree as well, since we might free > 2GB in one shot on i386 as well...

Could be. In most places it is used to free a single block. I suspect that the FFS_BLK_FREE sysctl handler can misbehave on 32-bit systems.