On Tue, Nov 15, 2016 at 12:53:35PM -0800, Kees Cook wrote:
On Tue, Nov 15, 2016 at 12:03 AM, Peter Zijlstra <peterz@xxxxxxxxxxxxx> wrote:
On Tue, Nov 15, 2016 at 08:33:22AM +0100, Greg KH wrote:
On Mon, Nov 14, 2016 at 06:39:48PM +0100, Peter Zijlstra wrote:
@@ -520,7 +520,7 @@ static void mod_rq_state(struct drbd_req
/* Completion does it's own kref_put. If we are going to
* kref_sub below, we need req to be still around then. */
int at_least = k_put + !!c_put;
- int refcount = atomic_read(&req->kref.refcount);
+ int refcount = kref_read(&req->kref);
if (refcount < at_least)
"mod_rq_state: Logic BUG: %x -> %x: refcount = %d, should be >= %d\n",
As proof of "things you should never do", here is one such example.
@@ -767,7 +767,7 @@ static void virtblk_remove(struct virtio
/* Stop all the virtqueues. */
- refc = atomic_read(&disk_to_dev(vblk->disk)->kobj.kref.refcount);
+ refc = kref_read(&disk_to_dev(vblk->disk)->kobj.kref);
And this too, ugh, that's a huge abuse and is probably totally wrong...
thanks again for digging through this crap. I wonder if we need to name
the kref reference variable "do_not_touch_this_ever" or some such thing
to catch all of the people who try to be "too smart".
There's unimaginable bong hits involved in this stuff, in the end I
resorted to brute force and scripts to convert all this.
What should we do about things like this (bpf_prog_put() and callbacks
static void bpf_prog_uncharge_memlock(struct bpf_prog *prog)
struct user_struct *user = prog->aux->user;
Oh that's scary. Let's just make one reference count rely on another
one and not check things...
static void __bpf_prog_put_rcu(struct rcu_head *rcu)
struct bpf_prog_aux *aux = container_of(rcu, struct bpf_prog_aux, rcu);
void bpf_prog_put(struct bpf_prog *prog)
Not only do we want to protect prog->aux->refcnt, but I think we want
to protect user->locked_vm too ... I don't think it's sane for
user->locked_vm to be a stats_t ?
I don't think this is sane code...