limits->max_sectors is getting set to 0, why/where? [was: Re: dm: kernel oops by divide error on v4.16+]

Commit Message

Kees Cook April 9, 2018, 10:38 p.m. UTC

On Mon, Apr 9, 2018 at 3:32 PM, Jens Axboe <[e-mail]> wrote:
> That's bad, for sure, but my worry was bigger than an oops or crash,
> we could have had corruption due to this.
>
> The resulting min/max and friends would have been trivial to test, but
> clearly they weren't.

Yeah, that was bad luck and my fault: I tested min(), max(), min_t(),
and max_t(). My assumption was that since the others were built from
them, they'd be fine. Not true in this shadow variable case, though.
:(  We could do something like this, which would have caught it:

Comments

Jens Axboe April 9, 2018, 11:01 p.m. UTC | #1

On 4/9/18 4:38 PM, Kees Cook wrote:
> On Mon, Apr 9, 2018 at 3:32 PM, Jens Axboe <[e-mail]> wrote:
>> That's bad, for sure, but my worry was bigger than an oops or crash,
>> we could have had corruption due to this.
>>
>> The resulting min/max and friends would have been trivial to test, but
>> clearly they weren't.
> 
> Yeah, that was bad luck and my fault: I tested min(), max(), min_t(),

It's only bad luck if it was tested :-)

> and max_t(). My assumption was that since the others were built from
> them, they'd be fine. Not true in this shadow variable case, though.
> :(  We could do something like this, which would have caught it:

Might not hurt to do.

Patch

diff --git a/init/main.c b/init/main.c
index e4a3160991ea..ce46afc53b8b 100644
--- a/init/main.c
+++ b/init/main.c
@@ -993,10 +993,32 @@  static inline void mark_readonly(void)
 }
 #endif

+static inline void compiletime_sanity_checks(void)
+{
+       /* Sanity-check min()/max() family of macros. */
+       BUILD_BUG_ON(min(5, 50) != 5);
+       BUILD_BUG_ON(max(5, 50) != 50);
+       BUILD_BUG_ON(min_t(int, (size_t)-1 , 50) != -1);
+       BUILD_BUG_ON(max_t(size_t, -1 , 50) != (size_t)-1);
+       BUILD_BUG_ON(min3(-50, 0, 1000) != -50);
+       BUILD_BUG_ON(max3(-50, 0, 1000) != 1000);
+       BUILD_BUG_ON(min_not_zero(0, 20) != 20);
+       BUILD_BUG_ON(min_not_zero(30, 0) != 30);
+       BUILD_BUG_ON(min_not_zero(150, 40) != 40);
+       BUILD_BUG_ON(clamp(20, 1, 7) != 7);
+       BUILD_BUG_ON(clamp(40, 20, 100) != 40);
+       BUILD_BUG_ON(clamp(1, 20, 100) != 20);
+       BUILD_BUG_ON(clamp_t(int, -5, (size_t)-1, 100) != -1);
+       BUILD_BUG_ON(clamp_t(int, -1, (size_t)-5, 100) != -1);
+       BUILD_BUG_ON(clamp_t(size_t, -10, 1, -50) != -50);
+}
+
 static int __ref kernel_init(void *unused)
 {
        int ret;

+       compiletime_sanity_checks();
+
        kernel_init_freeable();
        /* need to finish all async __init code before freeing the memory */
        async_synchronize_full();