Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1689315

RE: [PATCH 14/15] kernel: convert futex_pi_state.refcount from atomic_t to refcount_t

Show all headers | View raw

On Mon, 17 Jul 2017, Reshetova, Elena wrote:
> > On Mon, 17 Jul 2017, Elena Reshetova wrote:
> > > 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.
> > 
> > Copying the same sentence over and over avoids thinking about a proper
> > changelog, right? You fail to explain, how you come to the conclusion that
> > futex_pi_state.refcount is a pure reference counter (aside of the name) and
> > therefor can be safely converted to refcount_t.

> OK, this is not very useful for many cases. Yes, I am using automated log
> on these patches, because I used to have 240 of them and writing manual
> logs for them would be fun.

Been there, done that.

> Moreover, in many cases, writing manual logs don't bring any value since
> I would have to repeat the same things all over again: xyz conversions
> was found by using *.cocci pattern first, then looked at manually and it
> looked like a standard reference counter that frees the things after
> calling refcount_dec_and_test() (or its variation with lock which is
> rare).  Other things also looked correct, like I didn't see increments
> from zero, counter starts at 1 etc.  I would really have to repeat the
> same thing in each changelog. Does it really bring value?

You don't have to go into that level of detail, but you can provide enough
information with a template as well, e.g.:

   atomic_t variables are often used to implement pure reference counters:
     - starting at 1
     - freeing a resource after reaching zero
     - only using basic atomic operations (init, inc, dec_and_test)

   These variables should be converted to refcount_t because the refcount_t
   operations can catch and prevent accidental underflows and overflows.

   The variable FOO is used as a pure reference counter. Convert it to
   refcount_t and fix up the operations.

That gives enough context for someone who looks at a patch because then the
reviewer can look for:

   starts at 1, frees at 0, does not use any fancy operations

and has not to use Gurgle to figure out what your understanding of
reference counters is.

Replacing FOO with the real variable name can be done with a script easy
enough.

Thanks,

	tglx

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

[PATCH 00/15] v3 kernel core pieces refcount conversions Elena Reshetova <elena.reshetova@intel.com> - 2017-07-17 12:50 +0200
  [PATCH 07/15] kernel: convert ring_buffer.refcount from atomic_t to refcount_t Elena Reshetova <elena.reshetova@intel.com> - 2017-07-17 12:50 +0200
  [PATCH 02/15] kernel: convert signal_struct.sigcnt from atomic_t to refcount_t Elena Reshetova <elena.reshetova@intel.com> - 2017-07-17 12:50 +0200
  [PATCH 04/15] kernel: convert task_struct.usage from atomic_t to refcount_t Elena Reshetova <elena.reshetova@intel.com> - 2017-07-17 12:50 +0200
  [PATCH 13/15] sched: convert numa_group.refcount from atomic_t to refcount_t Elena Reshetova <elena.reshetova@intel.com> - 2017-07-17 12:50 +0200
  [PATCH 03/15] kernel: convert user_struct.__count from atomic_t to refcount_t Elena Reshetova <elena.reshetova@intel.com> - 2017-07-17 12:50 +0200
  [PATCH 09/15] kernel: convert uprobe.ref from atomic_t to refcount_t Elena Reshetova <elena.reshetova@intel.com> - 2017-07-17 12:50 +0200
  [PATCH 14/15] kernel: convert futex_pi_state.refcount from atomic_t to refcount_t Elena Reshetova <elena.reshetova@intel.com> - 2017-07-17 12:50 +0200
    Re: [PATCH 14/15] kernel: convert futex_pi_state.refcount from  atomic_t to refcount_t Thomas Gleixner <tglx@linutronix.de> - 2017-07-17 16:30 +0200
      RE: [PATCH 14/15] kernel: convert futex_pi_state.refcount from  atomic_t to refcount_t "Reshetova, Elena" <elena.reshetova@intel.com> - 2017-07-17 19:00 +0200
        RE: [PATCH 14/15] kernel: convert futex_pi_state.refcount from  atomic_t to refcount_t Thomas Gleixner <tglx@linutronix.de> - 2017-07-17 20:00 +0200
          RE: [PATCH 14/15] kernel: convert futex_pi_state.refcount from  atomic_t to refcount_t "Reshetova, Elena" <elena.reshetova@intel.com> - 2017-07-18 11:40 +0200
  [PATCH 15/15] kernel: convert kcov.refcount from atomic_t to refcount_t Elena Reshetova <elena.reshetova@intel.com> - 2017-07-17 12:50 +0200
  [PATCH 10/15] kernel: convert nsproxy.count from atomic_t to refcount_t Elena Reshetova <elena.reshetova@intel.com> - 2017-07-17 12:50 +0200
  [PATCH 11/15] kernel: convert group_info.usage from atomic_t to refcount_t Elena Reshetova <elena.reshetova@intel.com> - 2017-07-17 12:50 +0200
  [PATCH 01/15] kernel: convert sighand_struct.count from atomic_t to refcount_t Elena Reshetova <elena.reshetova@intel.com> - 2017-07-17 12:50 +0200
  [PATCH 05/15] kernel: convert task_struct.stack_refcount from atomic_t to refcount_t Elena Reshetova <elena.reshetova@intel.com> - 2017-07-17 12:50 +0200
  [PATCH 06/15] kernel: convert perf_event_context.refcount from atomic_t to refcount_t Elena Reshetova <elena.reshetova@intel.com> - 2017-07-17 12:50 +0200
  [PATCH 12/15] kernel: convert cred.usage from atomic_t to refcount_t Elena Reshetova <elena.reshetova@intel.com> - 2017-07-17 12:50 +0200

csiph-web