Re: [PATCH] percpu_counter: `percpu_counter_read_positive' should not return negative number

[Tejun Heo]

On Wed, Jun 06, 2018 at 09:39:40PM +0200, Mathieu Malaterre wrote:
> Since function `percpu_counter_add' may result in a signed integer overflow
> the result stored in `fbc->count' could be negative. Make sure that
> function `percpu_counter_read_positive' does not return a negative number
> in this case. This will match behavior when CONFIG_SMP=y.
> 
> Detected wth CONFIG_UBSAN=y
> 
> [76404.888450] ================================================================================
> [76404.888477] UBSAN: Undefined behaviour in ../include/linux/percpu_counter.h:136:13
> [76404.888485] signed integer overflow:
> [76404.888490] 9223308017647617321 + 76624449492175 cannot be represented in type 'long long int'
> [76404.888501] CPU: 0 PID: 0 Comm: swapper Not tainted 4.17.0+ #50
> [76404.888506] Call Trace:
> [76404.888523] [dffedd30] [c0478b90] ubsan_epilogue+0x18/0x4c (unreliable)
> [76404.888533] [dffedd40] [c0479530] handle_overflow+0xbc/0xdc
> [76404.888548] [dffeddc0] [c0439044] cfq_completed_request+0x560/0x1234
> [76404.888557] [dffede40] [c03f3fc4] __blk_put_request+0xb0/0x2dc
> [76404.888570] [dffede80] [c05a81d0] scsi_end_request+0x19c/0x344
> [76404.888579] [dffedeb0] [c05a9954] scsi_io_completion+0x4b4/0x854
> [76404.888592] [dffedf10] [c04046b4] blk_done_softirq+0xe4/0x1e0
> [76404.888605] [dffedf60] [c07ec1d4] __do_softirq+0x16c/0x5f0
> [76404.888617] [dffedfd0] [c0065160] irq_exit+0x110/0x1a8
> [76404.888629] [dffedff0] [c001646c] call_do_irq+0x24/0x3c
> [76404.888641] [c0cdbe80] [c0009a2c] do_IRQ+0x98/0x1a0
> [76404.888649] [c0cdbeb0] [c001b93c] ret_from_except+0x0/0x14
> [76404.888659] --- interrupt: 501 at arch_cpu_idle+0x30/0x78
>                    LR = arch_cpu_idle+0x30/0x78
> [76404.888667] [c0cdbf70] [c0cda000] 0xc0cda000 (unreliable)
> [76404.888679] [c0cdbf80] [c00a3844] do_idle+0xc4/0x158
> [76404.888687] [c0cdbfb0] [c00a3a90] cpu_startup_entry+0x24/0x28
> [76404.888696] [c0cdbfc0] [c097f820] start_kernel+0x47c/0x490
> [76404.888703] [c0cdbff0] [00003444] 0x3444

So, the _positive versions are there to deal with small negative reads
coming from percpu propagation delays.  It's not there to protect
against actual counter overflow although it can't detect that on SMP.
It's just outright wrong to report 0 when the counter actually
overflowed and applying the suggested patch masks a real problem
undetectable.

I think the right thing to do is actually undersatnding what's going
on (why is a 64bit counter overflowing?) and fix the underlying issue.

Thanks.

-- 
tejun