Skip to contentSkip to conversation

libceph: use 64-bit arithmetic instead of 32-bit

PATCH In review Gustavo A. R. Silva 3 participants 3 messages Jan 31, 2018

Subsystems (3)
Networking Network drivers Ceph common code (libceph)
lore
Share & tools
lore

Gustavo A. R. Silva

author
#0 lore raw

PATCH 1/1 libceph: use 64-bit arithmetic instead of 32-bit

Cast objsetno to u64 in order to give the compiler complete information about the proper arithmetic to use. Notice that this variable is used in a context that expects an expression of type u64 (64 bits, unsigned).

The expression objsetno * sc + stripepos is currently being evaluated using 32-bit arithmetic.

In general, the use of incorrect arithmetic has security implications.

Addresses-Coverity-ID: 200686

Signed-off-by Gustavo A. R. Silva <[email protected]>
---
 net/ceph/osdmap.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
net/ceph/osdmap.c +1 -1
index 0da27c6..58dc965 100644
@@ -2183,7 +2183,7 @@ int ceph_calc_file_object_mapping(struct ceph_file_layout *layout,
 	stripepos = bl % sc;
 	objsetno = stripeno / su_per_object;
 
-	*ono = objsetno * sc + stripepos;
+	*ono = (u64)objsetno * sc + stripepos;
 	dout("objset %u * sc %u = ono %u\n", objsetno, sc, (unsigned int)*ono);
 
 	/* *oxoff = *off % layout->fl_stripe_unit;  # offset in su */
Signature · 1 line
2.7.4
maintainer
#1 lore raw
Quoted 22 lines · Gustavo A. R. Silva

Cast objsetno to u64 in order to give the compiler complete information about the proper arithmetic to use. Notice that this variable is used in a context that expects an expression of type u64 (64 bits, unsigned).

The expression objsetno * sc + stripepos is currently being evaluated using 32-bit arithmetic.

In general, the use of incorrect arithmetic has security implications.

Addresses-Coverity-ID: 200686

Signed-off-by Gustavo A. R. Silva <[email protected]>
---
 net/ceph/osdmap.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
net/ceph/osdmap.c +0 -0
index 0da27c6..58dc965 100644

net/ceph/osdmap.c int ceph_calc_file_object_mapping(struct ceph_file_layout *layout, · line 2189

Quoted 8 lines
net/ceph/osdmap.c +1 -1
@@ -2183,7 +2183,7 @@ int ceph_calc_file_object_mapping(struct ceph_file_layout *layout,
        stripepos = bl % sc;
        objsetno = stripeno / su_per_object;
 
-       *ono = objsetno * sc + stripepos;
+       *ono = (u64)objsetno * sc + stripepos;
        dout("objset %u * sc %u = ono %u\n", objsetno, sc, (unsigned int)*ono);
 
        /* *oxoff = *off % layout->fl_stripe_unit;  # offset in su */

Hi Gustavo,

This (and other u32/u64 issues in this function, is this the only warning?) is fixed in my striping v2 work branch. I wasn't going to push that patch separately, but I guess I should post it.

Thanks,

Ilya

#2 lore raw

Hello Ilya,

Quoting Ilya Dryomov <[email protected]>:

Quoted 33 lines
Quoted 31 lines · Gustavo A. R. Silva

Cast objsetno to u64 in order to give the compiler complete information about the proper arithmetic to use. Notice that this variable is used in a context that expects an expression of type u64 (64 bits, unsigned).

The expression objsetno * sc + stripepos is currently being evaluated using 32-bit arithmetic.

In general, the use of incorrect arithmetic has security implications.

Addresses-Coverity-ID: 200686

Signed-off-by Gustavo A. R. Silva <[email protected]>
---
 net/ceph/osdmap.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
net/ceph/osdmap.c +1 -1
index 0da27c6..58dc965 100644
@@ -2183,7 +2183,7 @@ int ceph_calc_file_object_mapping(struct  
 ceph_file_layout *layout,
        stripepos = bl % sc;
        objsetno = stripeno / su_per_object;
 
-       *ono = objsetno * sc + stripepos;
+       *ono = (u64)objsetno * sc + stripepos;
        dout("objset %u * sc %u = ono %u\n", objsetno, sc,  
 (unsigned int)*ono);
Quoted 8 lines
Quoted 1 line
        /* *oxoff = *off % layout->fl_stripe_unit;  # offset in su */

Hi Gustavo,

This (and other u32/u64 issues in this function, is this the only warning?) is fixed in my striping v2 work branch. I wasn't going to push that patch separately, but I guess I should post it.

Yeah, this was the only one warning reported by Coverity in this particular module.

Thanks