[2649] | 1 | From 368fff3b2fef5b559ed6da016e1e9f0413b55552 Mon Sep 17 00:00:00 2001 |
---|
| 2 | From: Andrew Deason <adeason@sinenomine.net> |
---|
| 3 | Date: Sun, 14 Sep 2014 14:10:11 -0500 |
---|
| 4 | Subject: [PATCH] afs: Fix some afs_conn overcounts |
---|
| 5 | |
---|
| 6 | The usual pattern of using afs_Conn looks like this: |
---|
| 7 | |
---|
| 8 | do { |
---|
| 9 | tc = afs_Conn(...); |
---|
| 10 | if (tc) { |
---|
| 11 | code = /* ... */ |
---|
| 12 | } else { |
---|
| 13 | code = -1; |
---|
| 14 | } |
---|
| 15 | } while (afs_Analyze(...)); |
---|
| 16 | |
---|
| 17 | The afs_Analyze call, amongst other things, puts back the reference to |
---|
| 18 | the connection obtained from afs_Conn. If anything inside the do/while |
---|
| 19 | block exits that block without calling afs_Analyze or afs_PutConn, we |
---|
| 20 | will leak a reference to the conn. |
---|
| 21 | |
---|
| 22 | A few places currently do this, by jumping out of the loop with |
---|
| 23 | 'goto's. Specifically, in afs_dcache.c and afs_bypasscache.c. These |
---|
| 24 | locations currently leak references to our connection object (and to |
---|
| 25 | the underlying Rx connection object), which can cause problems over |
---|
| 26 | time. Specifically, this can cause a panic when the refcount overflows |
---|
| 27 | and becomes negative, causing a panic message that looks like: |
---|
| 28 | |
---|
| 29 | afs_PutConn: refcount imbalance 0xd34db33f -32768 |
---|
| 30 | |
---|
| 31 | To avoid this, make sure we afs_PutConn in these cases where we 'goto' |
---|
| 32 | out of the afs_Conn/afs_Analyze loop. Perhaps ideally we should cause |
---|
| 33 | afs_Analyze itself to be called in these situations, but for now just |
---|
| 34 | fix the problem with the least amount of impact possible. |
---|
| 35 | |
---|
| 36 | FIXES 131885 |
---|
| 37 | |
---|
| 38 | Change-Id: I3a52f8ccef24f01d04c02db0a4b711405360e323 |
---|
| 39 | --- |
---|
| 40 | src/afs/afs_bypasscache.c | 1 + |
---|
| 41 | src/afs/afs_dcache.c | 7 +++++++ |
---|
| 42 | 2 files changed, 8 insertions(+) |
---|
| 43 | |
---|
| 44 | diff --git a/src/afs/afs_bypasscache.c b/src/afs/afs_bypasscache.c |
---|
| 45 | index 744feb2..8fc6009 100644 |
---|
| 46 | --- a/src/afs/afs_bypasscache.c |
---|
| 47 | +++ b/src/afs/afs_bypasscache.c |
---|
| 48 | @@ -637,6 +637,7 @@ afs_PrefetchNoCache(struct vcache *avc, |
---|
| 49 | } else { |
---|
| 50 | afs_warn("BYPASS: StartRXAFS_FetchData failed: %d\n", code); |
---|
| 51 | unlock_and_release_pages(auio); |
---|
| 52 | + afs_PutConn(tc, rxconn, SHARED_LOCK); |
---|
| 53 | goto done; |
---|
| 54 | } |
---|
| 55 | if (code == 0) { |
---|
| 56 | diff --git a/src/afs/afs_dcache.c b/src/afs/afs_dcache.c |
---|
| 57 | index dc1e039..a156e22 100644 |
---|
| 58 | --- a/src/afs/afs_dcache.c |
---|
| 59 | +++ b/src/afs/afs_dcache.c |
---|
| 60 | @@ -2399,6 +2399,13 @@ afs_GetDCache(struct vcache *avc, afs_size_t abyte, |
---|
| 61 | afs_PutDCache(tdc); |
---|
| 62 | tdc = 0; |
---|
| 63 | ReleaseReadLock(&avc->lock); |
---|
| 64 | + |
---|
| 65 | + if (tc) { |
---|
| 66 | + /* If we have a connection, we must put it back, |
---|
| 67 | + * since afs_Analyze will not be called here. */ |
---|
| 68 | + afs_PutConn(tc, rxconn, SHARED_LOCK); |
---|
| 69 | + } |
---|
| 70 | + |
---|
| 71 | slowPass = 1; |
---|
| 72 | goto RetryGetDCache; |
---|
| 73 | } |
---|
| 74 | -- |
---|
| 75 | 2.2.0 |
---|
| 76 | |
---|