The caller of nfs4_find_slot() expects NFS4_MAX_SLOT_TABLE when no slot is
available (also mentioned in the comment header for nfs4_find_slot()).
Signed-off-by: Weston Andros Adamson <[email protected]>
---
This fixes an OOPS that is easy to reproduce:
dd if=/dev/zero of=./zerofile bs=10024 count=1
nfs4_find_slot() returns 2^32-1 which != NFS4_MAX_SLOT_TABLE, so it's used
as a valid slotid.
Note that NFS4_NO_SLOT is still used for nfs4_slot_table::highest_used_slotid
fs/nfs/nfs4proc.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index da985c3..b977fea 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -520,7 +520,7 @@ static u32
nfs4_find_slot(struct nfs4_slot_table *tbl)
{
u32 slotid;
- u32 ret_id = NFS4_NO_SLOT;
+ u32 ret_id = NFS4_MAX_SLOT_TABLE;
dprintk("--> %s used_slots=%04lx highest_used=%u max_slots=%u\n",
__func__, tbl->used_slots[0], tbl->highest_used_slotid,
@@ -533,7 +533,7 @@ nfs4_find_slot(struct nfs4_slot_table *tbl)
tbl->highest_used_slotid = slotid;
ret_id = slotid;
out:
- dprintk("<-- %s used_slots=%04lx highest_used=%d slotid=%d \n",
+ dprintk("<-- %s used_slots=%04lx highest_used=%u slotid=%u\n",
__func__, tbl->used_slots[0], tbl->highest_used_slotid, ret_id);
return ret_id;
}
--
1.7.4.4
On Feb 14, 2012, at 5:23 PM, Myklebust, Trond wrote:
> On Tue, 2012-02-14 at 16:23 -0500, Weston Andros Adamson wrote:
>> The caller of nfs4_find_slot() expects NFS4_MAX_SLOT_TABLE when no slot is
>> available (also mentioned in the comment header for nfs4_find_slot()).
>>
>> Signed-off-by: Weston Andros Adamson <[email protected]>
>> ---
>> This fixes an OOPS that is easy to reproduce:
>>
>> dd if=/dev/zero of=./zerofile bs=10024 count=1
>>
>> nfs4_find_slot() returns 2^32-1 which != NFS4_MAX_SLOT_TABLE, so it's used
>> as a valid slotid.
>>
>> Note that NFS4_NO_SLOT is still used for nfs4_slot_table::highest_used_slotid
>
> We need to fix the broken caller, not nfs4_find_slot()... I seem to have
> missed a couple of instances in the 'Convert slotid' patch?
>
> How's this?
Looks good - one minor issue below.
-dros
>
> 8<-----------------------------------------------------------------------------
> From 6e885e370726c4d72a24acfb06cbd6673e4b791f Mon Sep 17 00:00:00 2001
> From: Trond Myklebust <[email protected]>
> Date: Tue, 14 Feb 2012 17:11:57 -0500
> Subject: [PATCH] fixup! NFSv4.1: Convert slotid from u8 to u32
>
> Signed-off-by: Trond Myklebust <[email protected]>
> ---
> fs/nfs/nfs4proc.c | 21 ++++++++++-----------
> 1 files changed, 10 insertions(+), 11 deletions(-)
>
> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> index da985c3..0b33165 100644
> --- a/fs/nfs/nfs4proc.c
> +++ b/fs/nfs/nfs4proc.c
> @@ -362,16 +362,14 @@ static void renew_lease(const struct nfs_server *server, unsigned long timestamp
> * When updating highest_used_slotid there may be "holes" in the bitmap
> * so we need to scan down from highest_used_slotid to 0 looking for the now
> * highest slotid in use.
> - * If none found, highest_used_slotid is set to -1.
> + * If none found, highest_used_slotid is set to NFS4_NO_SLOT.
> *
> * Must be called while holding tbl->slot_tbl_lock
> */
> static void
> -nfs4_free_slot(struct nfs4_slot_table *tbl, u8 free_slotid)
> +nfs4_free_slot(struct nfs4_slot_table *tbl, u32 slotid)
> {
> - int slotid = free_slotid;
> -
> - BUG_ON(slotid < 0 || slotid >= NFS4_MAX_SLOT_TABLE);
> + BUG_ON(slotid >= NFS4_MAX_SLOT_TABLE);
> /* clear used bit in bitmap */
> __clear_bit(slotid, tbl->used_slots);
>
> @@ -381,10 +379,10 @@ nfs4_free_slot(struct nfs4_slot_table *tbl, u8 free_slotid)
> if (slotid < tbl->max_slots)
> tbl->highest_used_slotid = slotid;
> else
> - tbl->highest_used_slotid = -1;
> + tbl->highest_used_slotid = NFS4_NO_SLOT;
> }
> - dprintk("%s: free_slotid %u highest_used_slotid %d\n", __func__,
> - free_slotid, tbl->highest_used_slotid);
> + dprintk("%s: slotid %u highest_used_slotid %d\n", __func__,
> + slotid, tbl->highest_used_slotid);
%d -> %u, also %d -> %u in dprintk before return in nfs4_find_slot().
> }
>
> bool nfs4_set_task_privileged(struct rpc_task *task, void *dummy)
> @@ -512,7 +510,7 @@ static int nfs4_sequence_done(struct rpc_task *task,
> * nfs4_find_slot looks for an unset bit in the used_slots bitmap.
> * If found, we mark the slot as used, update the highest_used_slotid,
> * and respectively set up the sequence operation args.
> - * The slot number is returned if found, or NFS4_MAX_SLOT_TABLE otherwise.
> + * The slot number is returned if found, or NFS4_NO_SLOT otherwise.
> *
> * Note: must be called with under the slot_tbl_lock.
> */
> @@ -529,7 +527,8 @@ nfs4_find_slot(struct nfs4_slot_table *tbl)
> if (slotid >= tbl->max_slots)
> goto out;
> __set_bit(slotid, tbl->used_slots);
> - if (slotid > tbl->highest_used_slotid)
> + if (slotid > tbl->highest_used_slotid ||
> + tbl->highest_used_slotid == NFS4_NO_SLOT)
> tbl->highest_used_slotid = slotid;
> ret_id = slotid;
> out:
> @@ -584,7 +583,7 @@ int nfs41_setup_sequence(struct nfs4_session *session,
> }
>
> slotid = nfs4_find_slot(tbl);
> - if (slotid == NFS4_MAX_SLOT_TABLE) {
> + if (slotid == NFS4_NO_SLOT) {
> rpc_sleep_on(&tbl->slot_tbl_waitq, task, NULL);
> spin_unlock(&tbl->slot_tbl_lock);
> dprintk("<-- %s: no free slots\n", __func__);
> --
> 1.7.7.6
>
>
> --
> Trond Myklebust
> Linux NFS client maintainer
>
> NetApp
> [email protected]
> http://www.netapp.com
>
T24gVHVlLCAyMDEyLTAyLTE0IGF0IDE2OjIzIC0wNTAwLCBXZXN0b24gQW5kcm9zIEFkYW1zb24g
d3JvdGU6DQo+IFRoZSBjYWxsZXIgb2YgbmZzNF9maW5kX3Nsb3QoKSBleHBlY3RzIE5GUzRfTUFY
X1NMT1RfVEFCTEUgd2hlbiBubyBzbG90IGlzDQo+IGF2YWlsYWJsZSAoYWxzbyBtZW50aW9uZWQg
aW4gdGhlIGNvbW1lbnQgaGVhZGVyIGZvciBuZnM0X2ZpbmRfc2xvdCgpKS4NCj4gDQo+IFNpZ25l
ZC1vZmYtYnk6IFdlc3RvbiBBbmRyb3MgQWRhbXNvbiA8ZHJvc0BuZXRhcHAuY29tPg0KPiAtLS0N
Cj4gVGhpcyBmaXhlcyBhbiBPT1BTIHRoYXQgaXMgZWFzeSB0byByZXByb2R1Y2U6DQo+IA0KPiAg
ZGQgaWY9L2Rldi96ZXJvIG9mPS4vemVyb2ZpbGUgYnM9MTAwMjQgY291bnQ9MQ0KPiANCj4gbmZz
NF9maW5kX3Nsb3QoKSByZXR1cm5zIDJeMzItMSB3aGljaCAhPSBORlM0X01BWF9TTE9UX1RBQkxF
LCBzbyBpdCdzIHVzZWQNCj4gYXMgYSB2YWxpZCBzbG90aWQuDQo+IA0KPiBOb3RlIHRoYXQgTkZT
NF9OT19TTE9UIGlzIHN0aWxsIHVzZWQgZm9yIG5mczRfc2xvdF90YWJsZTo6aGlnaGVzdF91c2Vk
X3Nsb3RpZA0KDQpXZSBuZWVkIHRvIGZpeCB0aGUgYnJva2VuIGNhbGxlciwgbm90IG5mczRfZmlu
ZF9zbG90KCkuLi4gSSBzZWVtIHRvIGhhdmUNCm1pc3NlZCBhIGNvdXBsZSBvZiBpbnN0YW5jZXMg
aW4gdGhlICdDb252ZXJ0IHNsb3RpZCcgcGF0Y2guLi4NCg0KSG93J3MgdGhpcz8NCg0KODwtLS0t
LS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0t
LS0tLS0tLS0tLS0tLS0tLQ0KRnJvbSA2ZTg4NWUzNzA3MjZjNGQ3MmEyNGFjZmIwNmNiZDY2NzNl
NGI3OTFmIE1vbiBTZXAgMTcgMDA6MDA6MDAgMjAwMQ0KRnJvbTogVHJvbmQgTXlrbGVidXN0IDxU
cm9uZC5NeWtsZWJ1c3RAbmV0YXBwLmNvbT4NCkRhdGU6IFR1ZSwgMTQgRmViIDIwMTIgMTc6MTE6
NTcgLTA1MDANClN1YmplY3Q6IFtQQVRDSF0gZml4dXAhIE5GU3Y0LjE6IENvbnZlcnQgc2xvdGlk
IGZyb20gdTggdG8gdTMyDQoNClNpZ25lZC1vZmYtYnk6IFRyb25kIE15a2xlYnVzdCA8VHJvbmQu
TXlrbGVidXN0QG5ldGFwcC5jb20+DQotLS0NCiBmcy9uZnMvbmZzNHByb2MuYyB8ICAgMjEgKysr
KysrKysrKy0tLS0tLS0tLS0tDQogMSBmaWxlcyBjaGFuZ2VkLCAxMCBpbnNlcnRpb25zKCspLCAx
MSBkZWxldGlvbnMoLSkNCg0KZGlmZiAtLWdpdCBhL2ZzL25mcy9uZnM0cHJvYy5jIGIvZnMvbmZz
L25mczRwcm9jLmMNCmluZGV4IGRhOTg1YzMuLjBiMzMxNjUgMTAwNjQ0DQotLS0gYS9mcy9uZnMv
bmZzNHByb2MuYw0KKysrIGIvZnMvbmZzL25mczRwcm9jLmMNCkBAIC0zNjIsMTYgKzM2MiwxNCBA
QCBzdGF0aWMgdm9pZCByZW5ld19sZWFzZShjb25zdCBzdHJ1Y3QgbmZzX3NlcnZlciAqc2VydmVy
LCB1bnNpZ25lZCBsb25nIHRpbWVzdGFtcA0KICAqIFdoZW4gdXBkYXRpbmcgaGlnaGVzdF91c2Vk
X3Nsb3RpZCB0aGVyZSBtYXkgYmUgImhvbGVzIiBpbiB0aGUgYml0bWFwDQogICogc28gd2UgbmVl
ZCB0byBzY2FuIGRvd24gZnJvbSBoaWdoZXN0X3VzZWRfc2xvdGlkIHRvIDAgbG9va2luZyBmb3Ig
dGhlIG5vdw0KICAqIGhpZ2hlc3Qgc2xvdGlkIGluIHVzZS4NCi0gKiBJZiBub25lIGZvdW5kLCBo
aWdoZXN0X3VzZWRfc2xvdGlkIGlzIHNldCB0byAtMS4NCisgKiBJZiBub25lIGZvdW5kLCBoaWdo
ZXN0X3VzZWRfc2xvdGlkIGlzIHNldCB0byBORlM0X05PX1NMT1QuDQogICoNCiAgKiBNdXN0IGJl
IGNhbGxlZCB3aGlsZSBob2xkaW5nIHRibC0+c2xvdF90YmxfbG9jaw0KICAqLw0KIHN0YXRpYyB2
b2lkDQotbmZzNF9mcmVlX3Nsb3Qoc3RydWN0IG5mczRfc2xvdF90YWJsZSAqdGJsLCB1OCBmcmVl
X3Nsb3RpZCkNCituZnM0X2ZyZWVfc2xvdChzdHJ1Y3QgbmZzNF9zbG90X3RhYmxlICp0YmwsIHUz
MiBzbG90aWQpDQogew0KLQlpbnQgc2xvdGlkID0gZnJlZV9zbG90aWQ7DQotDQotCUJVR19PTihz
bG90aWQgPCAwIHx8IHNsb3RpZCA+PSBORlM0X01BWF9TTE9UX1RBQkxFKTsNCisJQlVHX09OKHNs
b3RpZCA+PSBORlM0X01BWF9TTE9UX1RBQkxFKTsNCiAJLyogY2xlYXIgdXNlZCBiaXQgaW4gYml0
bWFwICovDQogCV9fY2xlYXJfYml0KHNsb3RpZCwgdGJsLT51c2VkX3Nsb3RzKTsNCiANCkBAIC0z
ODEsMTAgKzM3OSwxMCBAQCBuZnM0X2ZyZWVfc2xvdChzdHJ1Y3QgbmZzNF9zbG90X3RhYmxlICp0
YmwsIHU4IGZyZWVfc2xvdGlkKQ0KIAkJaWYgKHNsb3RpZCA8IHRibC0+bWF4X3Nsb3RzKQ0KIAkJ
CXRibC0+aGlnaGVzdF91c2VkX3Nsb3RpZCA9IHNsb3RpZDsNCiAJCWVsc2UNCi0JCQl0YmwtPmhp
Z2hlc3RfdXNlZF9zbG90aWQgPSAtMTsNCisJCQl0YmwtPmhpZ2hlc3RfdXNlZF9zbG90aWQgPSBO
RlM0X05PX1NMT1Q7DQogCX0NCi0JZHByaW50aygiJXM6IGZyZWVfc2xvdGlkICV1IGhpZ2hlc3Rf
dXNlZF9zbG90aWQgJWRcbiIsIF9fZnVuY19fLA0KLQkJZnJlZV9zbG90aWQsIHRibC0+aGlnaGVz
dF91c2VkX3Nsb3RpZCk7DQorCWRwcmludGsoIiVzOiBzbG90aWQgJXUgaGlnaGVzdF91c2VkX3Ns
b3RpZCAlZFxuIiwgX19mdW5jX18sDQorCQlzbG90aWQsIHRibC0+aGlnaGVzdF91c2VkX3Nsb3Rp
ZCk7DQogfQ0KIA0KIGJvb2wgbmZzNF9zZXRfdGFza19wcml2aWxlZ2VkKHN0cnVjdCBycGNfdGFz
ayAqdGFzaywgdm9pZCAqZHVtbXkpDQpAQCAtNTEyLDcgKzUxMCw3IEBAIHN0YXRpYyBpbnQgbmZz
NF9zZXF1ZW5jZV9kb25lKHN0cnVjdCBycGNfdGFzayAqdGFzaywNCiAgKiBuZnM0X2ZpbmRfc2xv
dCBsb29rcyBmb3IgYW4gdW5zZXQgYml0IGluIHRoZSB1c2VkX3Nsb3RzIGJpdG1hcC4NCiAgKiBJ
ZiBmb3VuZCwgd2UgbWFyayB0aGUgc2xvdCBhcyB1c2VkLCB1cGRhdGUgdGhlIGhpZ2hlc3RfdXNl
ZF9zbG90aWQsDQogICogYW5kIHJlc3BlY3RpdmVseSBzZXQgdXAgdGhlIHNlcXVlbmNlIG9wZXJh
dGlvbiBhcmdzLg0KLSAqIFRoZSBzbG90IG51bWJlciBpcyByZXR1cm5lZCBpZiBmb3VuZCwgb3Ig
TkZTNF9NQVhfU0xPVF9UQUJMRSBvdGhlcndpc2UuDQorICogVGhlIHNsb3QgbnVtYmVyIGlzIHJl
dHVybmVkIGlmIGZvdW5kLCBvciBORlM0X05PX1NMT1Qgb3RoZXJ3aXNlLg0KICAqDQogICogTm90
ZTogbXVzdCBiZSBjYWxsZWQgd2l0aCB1bmRlciB0aGUgc2xvdF90YmxfbG9jay4NCiAgKi8NCkBA
IC01MjksNyArNTI3LDggQEAgbmZzNF9maW5kX3Nsb3Qoc3RydWN0IG5mczRfc2xvdF90YWJsZSAq
dGJsKQ0KIAlpZiAoc2xvdGlkID49IHRibC0+bWF4X3Nsb3RzKQ0KIAkJZ290byBvdXQ7DQogCV9f
c2V0X2JpdChzbG90aWQsIHRibC0+dXNlZF9zbG90cyk7DQotCWlmIChzbG90aWQgPiB0YmwtPmhp
Z2hlc3RfdXNlZF9zbG90aWQpDQorCWlmIChzbG90aWQgPiB0YmwtPmhpZ2hlc3RfdXNlZF9zbG90
aWQgfHwNCisJCQl0YmwtPmhpZ2hlc3RfdXNlZF9zbG90aWQgPT0gTkZTNF9OT19TTE9UKQ0KIAkJ
dGJsLT5oaWdoZXN0X3VzZWRfc2xvdGlkID0gc2xvdGlkOw0KIAlyZXRfaWQgPSBzbG90aWQ7DQog
b3V0Og0KQEAgLTU4NCw3ICs1ODMsNyBAQCBpbnQgbmZzNDFfc2V0dXBfc2VxdWVuY2Uoc3RydWN0
IG5mczRfc2Vzc2lvbiAqc2Vzc2lvbiwNCiAJfQ0KIA0KIAlzbG90aWQgPSBuZnM0X2ZpbmRfc2xv
dCh0YmwpOw0KLQlpZiAoc2xvdGlkID09IE5GUzRfTUFYX1NMT1RfVEFCTEUpIHsNCisJaWYgKHNs
b3RpZCA9PSBORlM0X05PX1NMT1QpIHsNCiAJCXJwY19zbGVlcF9vbigmdGJsLT5zbG90X3RibF93
YWl0cSwgdGFzaywgTlVMTCk7DQogCQlzcGluX3VubG9jaygmdGJsLT5zbG90X3RibF9sb2NrKTsN
CiAJCWRwcmludGsoIjwtLSAlczogbm8gZnJlZSBzbG90c1xuIiwgX19mdW5jX18pOw0KLS0gDQox
LjcuNy42DQoNCg0KLS0gDQpUcm9uZCBNeWtsZWJ1c3QNCkxpbnV4IE5GUyBjbGllbnQgbWFpbnRh
aW5lcg0KDQpOZXRBcHANClRyb25kLk15a2xlYnVzdEBuZXRhcHAuY29tDQp3d3cubmV0YXBwLmNv
bQ0KDQo=