On Thu, Jun 29, 2017 at 7:42 PM, Li Kun [off-list ref] wrote:

on 2017/6/30 6:05, Kees Cook wrote:

quoted

On Wed, Jun 28, 2017 at 9:13 PM, Li Kun [off-list ref] wrote:

quoted

在 2017/5/31 5:39, Kees Cook 写道:

quoted

+bool ex_handler_refcount(const struct exception_table_entry *fixup,
+                        struct pt_regs *regs, int trapnr)
+{
+       int reset;
+
+       /*
+        * If we crossed from INT_MAX to INT_MIN, the OF flag (result
+        * wrapped around) and the SF flag (result is negative) will be
+        * set. In this case, reset to INT_MAX in an attempt to leave
the
+        * refcount usable. Otherwise, we've landed here due to
producing
+        * a negative result from either decrementing zero or operating
on
+        * a negative value. In this case things are badly broken, so we
+        * we saturate to INT_MIN / 2.
+        */
+       if (regs->flags & (X86_EFLAGS_OF | X86_EFLAGS_SF))
+               reset = INT_MAX;

     Should it be like this to indicate that the refcount is wapped from
INT_MAX to INT_MIN ?
         if (regs->flags & (X86_EFLAGS_OF | X86_EFLAGS_SF)
                 == (X86_EFLAGS_OF | X86_EFLAGS_SF))

                 reset = INT_MAX;

Ah yes, thanks for the catch. Yeah, that test is expecting both
condition flags to be set.

I'm still on the fence about the best way to deal with the bad states.
I've been pondering just strictly using a saturation value (INT_MIN /
2), which should offer the same system state protection (except for
the inherent resource leak), but that means there isn't really a good
way to kill an offending process (since after saturation ALL processes
will look like violators). It can be argued that killing the process
doesn't actually provide any benefit since the system is still safe,
though.

An immature idea,can we set the count to INT_MAX/2 when we detect and kill
the offending process,
and wait to see if there will be another offender touching the fence. Er,not
very acurate,but better than
killing all the processes doing refcount_add ,i think.

quoted

quoted

quoted

+       else
+               reset = INT_MIN / 2;
+       *(int *)regs->cx = reset;

I suppose we could kill a process if it did the wrap from INT_MAX to
INT_MIN, and then ignore (though maintain saturation of) the rest.
i.e. if X86_EFLAGS_OF, kill and saturate. If X86_EFLAGS_SF, saturate.
I'm still curious about catching refcount_dec() (not
refcount_dec_and_test()) hitting zero.

-Kees

-- 
Kees Cook
Pixel Security