On Tue, Jun 06, 2017 at 07:47:55PM +0200, Jason A. Donenfeld wrote: > -static inline void key_alloc_serial(struct key *key) > +static inline int key_alloc_serial(struct key *key) > @@ -170,7 +168,7 @@ static inline void key_alloc_serial(struct key *key) > rb_insert_color(&key->serial_node, &key_serial_tree); > > spin_unlock(&key_serial_lock); > - return; > + return 0; > > /* we found a key with the proposed serial number - walk the tree from > * that point looking for the next unused serial number */ > @@ -314,7 +312,9 @@ struct key *key_alloc(struct key_type *type, const char *desc, > > /* publish the key by giving it a serial number */ > atomic_inc(&user->nkeys); > - key_alloc_serial(key); > + ret = key_alloc_serial(key); > + if (ret < 0) > + goto security_error; > > error: > return key; I'm guessing you changed key_alloc_serial() to return an int back when you were thinking that you might use get_random_bytes_wait(), which could return -ERESTARTSYS. Now that you're not doing this, but using get_random_u32() instead, there's no point to change the function signature of key_alloc_serial() and add an error check in key_alloc() that will never fail, right? That's just adding a dead code path. Which the compiler can probably optimize away, but why make the code slightly harder to read than necessasry? - Ted
