Ceph » rbd

Tried to reproduce, but couldn't. Looks to me like missing '\0' termination. Do you get this from all of your 0.67.7 machines?

My earlier diagnsis 0.67.4 vs 0.67.7 was incorrect -- actually that was a el6 vs ubuntu difference.

On my ubuntu 0.67.7 client, I cannot reproduce.

On all of my el6 clients, I can reproduce.

I will work backwords now to see if this was introduced sometime in dumpling or if it's something else.

Dan van der Ster wrote:

My earlier diagnsis 0.67.4 vs 0.67.7 was incorrect -- actually that was a el6 vs ubuntu difference.

On my ubuntu 0.67.7 client, I cannot reproduce.

On all of my el6 0.67.7 clients, I can reproduce.

I will work backwords now to see if this was introduced sometime in dumpling or if it's something else.

So this is not a recent regression. I can reproduce on el6.5 with 0.67-0

[root@dvtest1 ceph]# ceph --version
ceph version 0.67 (e3b7bc5bce8ab330ec1661381072368af3c218a0)
[root@dvtest1 ceph]# rbd -m cephmon info volumes/volume-f529978c-0981-4eba-a5b5-7ba8ecc05e1b
2014-03-11 14:39:54.963784 7fcfa8005760 -1 did not load config file, using default settings.
rbd image 'volume-f529978c-0981-4eba-a5b5-7ba8ecc05e1b':
size 5120 MB in 1280 objects
order 22 (4096 KB objects)
block_name_prefix: rbd_data.1000cf52ae8944a?
format: 2
features: layering
[root@dvtest1 ceph]# rbd -m cephmon info volumes/volume-f529978c-0981-4eba-a5b5-7ba8ecc05e1b
2014-03-11 14:39:55.723764 7fe36039e760 -1 did not load config file, using default settings.
rbd image 'volume-f529978c-0981-4eba-a5b5-7ba8ecc05e1b':
size 5120 MB in 1280 objects
order 22 (4096 KB objects)
block_name_prefix: rbd_data.1000cf52ae8944a?
format: 2
features: layering
[root@dvtest1 ceph]# rbd -m cephmon info volumes/volume-f529978c-0981-4eba-a5b5-7ba8ecc05e1b
2014-03-11 14:39:56.372348 7f27c13fe760 -1 did not load config file, using default settings.
rbd image 'volume-f529978c-0981-4eba-a5b5-7ba8ecc05e1b':
size 5120 MB in 1280 objects
order 22 (4096 KB objects)
block_name_prefix: rbd_data.1000cf52ae8944a'
format: 2
features: layering
[root@dvtest1 ceph]# rbd -m cephmon info volumes/volume-f529978c-0981-4eba-a5b5-7ba8ecc05e1b
2014-03-11 14:39:56.900712 7fe11152e760 -1 did not load config file, using default settings.
rbd image 'volume-f529978c-0981-4eba-a5b5-7ba8ecc05e1b':
size 5120 MB in 1280 objects
order 22 (4096 KB objects)
block_name_prefix: rbd_data.1000cf52ae8944a?
format: 2
features: layering
[root@dvtest1 ceph]# rbd -m cephmon info volumes/volume-f529978c-0981-4eba-a5b5-7ba8ecc05e1b
2014-03-11 14:39:57.491803 7fe7579f5760 -1 did not load config file, using default settings.
rbd image 'volume-f529978c-0981-4eba-a5b5-7ba8ecc05e1b':
size 5120 MB in 1280 objects
order 22 (4096 KB objects)
block_name_prefix: rbd_data.1000cf52ae8944a?
format: 2
features: layering

Here's a bit more with json format:

{"name":"volume-f529978c-0981-4eba-a5b5-7ba8ecc05e1b","size":5368709120,"objects":1280,"order":22,"object_size":4194304,"block_name_prefix":"rbd_data.1000cf52ae8944a?\u007f","format":2,"features":["layering","striping"]}[root@dvtest1 ~]#
[root@dvtest1 ~]# rbd -m cephmon info volumes/volume-f529978c-0981-4eba-a5b5-7ba8ecc05e1b --format json {"name":"volume-f529978c-0981-4eba-a5b5-7ba8ecc05e1b","size":5368709120,"objects":1280,"order":22,"object_size":4194304,"block_name_prefix":"rbd_data.1000cf52ae8944a?\u007f","format":2,"features":["layering","striping"]}[root@dvtest1 ~]# rbd -m cephmon info volumes/volume-f529978c-0981-4eba-a5b5-7ba8ecc05e1b --format json {"name":"volume-f529978c-0981-4eba-a5b5-7ba8ecc05e1b","size":5368709120,"objects":1280,"order":22,"object_size":4194304,"block_name_prefix":"rbd_data.1000cf52ae8944aP\u007f","format":2,"features":["layering","striping"]}[root@dvtest1 ~]# rbd -m cephmon info volumes/volume-f529978c-0981-4eba-a5b5-7ba8ecc05e1b --format json {"name":"volume-f529978c-0981-4eba-a5b5-7ba8ecc05e1b","size":5368709120,"objects":1280,"order":22,"object_size":4194304,"block_name_prefix":"rbd_data.1000cf52ae8944a\\\u007f","format":2,"features":["layering","striping"]}[root@dvtest1 ~]# rbd -m cephmon info volumes/volume-f529978c-0981-4eba-a5b5-7ba8ecc05e1b --format json {"name":"volume-f529978c-0981-4eba-a5b5-7ba8ecc05e1b","size":5368709120,"objects":1280,"order":22,"object_size":4194304,"block_name_prefix":"rbd_data.1000cf52ae8944a?\u007f","format":2,"features":["layering","striping"]}[root@dvtest1 ~]# rbd -m cephmon info volumes/volume-f529978c-0981-4eba-a5b5-7ba8ecc05e1b --format json {"name":"volume-f529978c-0981-4eba-a5b5-7ba8ecc05e1b","size":5368709120,"objects":1280,"order":22,"object_size":4194304,"block_name_prefix":"rbd_data.1000cf52ae8944ae\u007f","format":2,"features":["layering","striping"]}[root@dvtest1 ~]# rbd -m cephmon info volumes/volume-f529978c-0981-4eba-a5b5-7ba8ecc05e1b --format json {"name":"volume-f529978c-0981-4eba-a5b5-7ba8ecc05e1b","size":5368709120,"objects":1280,"order":22,"object_size":4194304,"block_name_prefix":"rbd_data.1000cf52ae8944a?\u007f","format":2,"features":["layering","striping"]}[root@dvtest1 ~]#

Sent this in email, but: u007f sounds like maybe a 0x7f character?.. Which should be legal 7-bit ASCII, but perhaps in some other encoding, it's not? Ceph isn't
very careful about Unicode.

I'm not current on my c++, but I'm pretty sure Danny's correct. Looking at the code, rbd.cc just does a cout of the 24-byte not-null-terminated char array. Perhaps that's not safe on rhel?

True, I don't see any attempt to add a nul. Oddly, it looks like there was half a plan to do so in librbd/internal.cc:image_info, but no actual NUL placed, and
it won't work for exactly-RBD_MAX_BLOCK_NAME_SIZE names anyway. blech.

Why I feel some guilt here: :)

commit 9a45ffb769c7c821196a8471009d0f9a4216c0d4
Author: Dan Mick <dan.mick@inktank.com>

I just rebuilt from current git dumpling head on my machine -- cannot reproduce. So I wonder this is something subtle about your build env vs. my running env.

Dan Mick wrote:

Why I feel some guilt here: :)

commit 9a45ffb769c7c821196a8471009d0f9a4216c0d4
Author: Dan Mick <dan.mick@inktank.com>

I've seen this commit. At my setup it doesn't cause any trouble and it was also in before 0.67.4.

Yeah, but I think it's data-dependent; must be max len and not have a zero byte directly after...

Do you take a look at it (or do you have already a fix?) or should I?

The problem is that block_name_prefix in rbd_image_info_t should be [RBD_MAX_BLOCK_NAME_SIZE+1] if you want to get a 24 char string as it currently is. An we need to add a '\0' if there is none copied from ictx->object_prefix.

But this would change librbd or we simply change the block_name_prefix to have max. 23 usable chars + '\0'.

The other solution would be to copy block_name_prefix to a local null terminated in the function where it is printed out, or?

Dan van der Ster wrote:

The other solution would be to copy block_name_prefix to a local null terminated in the function where it is printed out, or?

You could do that, but since this is a object from the librbd function, I would prefer to fix it directly in the librbd code instead since then all existing code that uses librbd would work fine.

Danny Al-Gaaf wrote:

Dan van der Ster wrote:

The other solution would be to copy block_name_prefix to a local null terminated in the function where it is printed out, or?

You could do that, but since this is a object from the librbd function, I would prefer to fix it directly in the librbd code instead since then all existing code that uses librbd would work fine.

OK, I was worried that other uses of the field would possibly copy the \0 into the actual prefix to access objects.

Dan van der Ster wrote:

Danny Al-Gaaf wrote:

Dan van der Ster wrote:

The other solution would be to copy block_name_prefix to a local null terminated in the function where it is printed out, or?

You could do that, but since this is a object from the librbd function, I would prefer to fix it directly in the librbd code instead since then all existing code that uses librbd would work fine.

OK, I was worried that other uses of the field would possibly copy the \0 into the actual prefix to access objects.

hm ... good question. I don't know what other code does here and I'm currently not so sure what you can expect from a char[] ... can you expect a \0 terminated string? RBD_MAX_BLOCK_NAME_SIZE suggests to me the name could also be less than 24 but in this case it would need to be \0 terminated to work or not?

I'm not working on this, just calling imprecations from the bleachers. :) I think the issue is that, if the string is <= 23 chars, the copy-into-output-struct copies the NUL, but if it's 24 or greater, the NUL is not copied. And you would only notice the bug if string+25 is also something other than NUL that prints in a way you'd notice, so it depends on the state of memory at the time.

The right fix is clearly to allocate 24+1 for the output struct and ensure the NUL is added on output.

Dan Mick wrote:

I'm not working on this, just calling imprecations from the bleachers. :) I think the issue is that, if the string is <= 23 chars, the copy-into-output-struct copies the NUL, but if it's 24 or greater, the NUL is not copied. And you would only notice the bug if string+25 is also something other than NUL that prints in a way you'd notice, so it depends on the state of memory at the time.

The right fix is clearly to allocate 24+1 for the output struct and ensure the NUL is added on output.

Only to be clear: you would propose to block_name_prefix/RBD_MAX_BLOCK_NAME_SIZE in librbd.h to 25 chars. We need to check if this affects API/ABI.

• rbd_obj_header_ondisk: Layout of parameter's stack of several functions has been changed and therefore parameters at higher positions in the stack may be incorrectly initialized by applications.
• ImageCtx:

1. The class has only inline or auto-generated constructors which will be copied to applications at compile time and will allocate an older memory layout. Call of any exported method of this class may access a memory outside the allocated objects or inside the older memory structure and result in crash or incorrect behavior of applications.
2. 2) The memory layout and size of subclasses will be changed.

• rbd_image_info_t: Type of field block_name_prefix has been changed from char²⁴ (24 bytes) to char²⁵ (25 bytes). This field may be incorrectly initialized or accessed by applications.

Question: is a ABI break acceptable? Or should we change it to 23+'\0' instead?