Re: [PATCH 1/3] ipc: convert ipc_namespace.count from atomic_t to refcount_t

[Eric W. Biederman]

"Reshetova, Elena" <elena.reshetova@xxxxxxxxx> writes:

2>> Elena Reshetova <elena.reshetova@xxxxxxxxx> writes:
>> 
>> > refcount_t type and corresponding API should be
>> > used instead of atomic_t when the variable is used as
>> > a reference counter. This allows to avoid accidental
>> > refcounter overflows that might lead to use-after-free
>> > situations.
>> 
>> In this patch you can see all of the uses of the count.
>> What accidental refcount overflows are possible?
>
> Even if one can guarantee and prove that in the current implementation
> there are no overflows possible, we can't say that for
> sure for any future implementation. Bugs might always happen
> unfortunately, but if we convert the refcounter to a safer
> type we can be sure that overflows are not possible. 
>
> Does it make sense to you?

Not for code that is likely to remain unchanged for a decade no.

This looks like a large set of unautomated changes without any real
thought put into it.  That almost always results in a typo somewhere
that breaks things.

So there is no benefit to the code, and a non-zero chance that there
will be a typo breaking the code.

All to harden the code for an unlikely future when the code is
updated with a full test cycle and people paying attention.

Introduce a bug now to avoid a bug in the future.  That seems like a
very poor engineering trade off.

Eric