Use bigger nodestr to accomodate 32-bit node numbers
PATCH 1/1 Use bigger nodestr to accomodate 32-bit node numbers
While using pacemaker/corosync, the node numbers are generated using IP address as opposed to serial node number generation. This may not fit in a 8-byte string. Use a bigger string to print the complete node number.
---
fs/ocfs2/super.c +1 -1
@@ -1022,7 +1022,7 @@ static int ocfs2_fill_super(struct super_block *sb, void *data, int silent)struct inode *inode =NULL ;struct ocfs2_super *osb =NULL ;struct buffer_head *bh =NULL ; -char nodestr[8 ]; +char nodestr[12 ];struct ocfs2_blockcheck_stats stats;trace_ocfs2_fill_super (sb, data, silent);
Signature · 1 line
Goldwyn
PATCH 1/1 Use bigger nodestr to accomodate 32-bit node numbers
Hi Goldwyn,
Quoted 4 lines · Goldwyn Rodrigues
While using pacemaker/corosync, the node numbers are generated using IP address as opposed to serial node number generation. This may not fit in a 8-byte string. Use a bigger string to print the complete node number.
I have no experience of pacemaker/corosync, so here is my comments pointing against this change only if we would expand the nodestr. :)
fs/ocfs2/super.c static int ocfs2_fill_super(struct super_block *sb, void *data, int silent) · line 1025
Quoted 13 lines
Signed-off-by Goldwyn Rodrigues <[email protected]>---
fs/ocfs2/super.c +1
-1@@ -1022,7 +1022,7 @@ static int ocfs2_fill_super(struct super_block *sb, void *data, int silent)struct inode *inode =NULL ;struct ocfs2_super *osb =NULL ;struct buffer_head *bh =NULL ; -char nodestr[8 ]; +char nodestr[12 ];
We have the corresponding debugging logic at ocfs2_dismount_volume(), which defines the nodestr with the old size, it seems that this change would cause potential buffer overflow while restore a bigger nodestr at it.
Also, there might has another relationship to ocfs2_control_message_setn/down at stack_user.c because we limit the length of nodenum with a marco:
#define OCFS2_CONTROL_MESSAGE_NODENUM_LEN 8
Maybe we should unify them?
Thanks,
-Jeff
PATCH 1/1 Use bigger nodestr to accomodate 32-bit node numbers
Quoted 13 lines · Jeff Liu
Hi Goldwyn,
Quoted 4 lines · Goldwyn Rodrigues
While using pacemaker/corosync, the node numbers are generated using IP address as opposed to serial node number generation. This may not fit in a 8-byte string. Use a bigger string to print the complete node number.
I have no experience of pacemaker/corosync, so here is my comments pointing against this change only if we would expand the nodestr. :)
Quoted 1 line
Quoted 16 lines
fs/ocfs2/super.c static int ocfs2_fill_super(struct super_block *sb, void *data, int silent) · line 1025
Quoted 12 lines
Signed-off-by Goldwyn Rodrigues <[email protected]>---
fs/ocfs2/super.c +1
-1@@ -1022,7 +1022,7 @@ static int ocfs2_fill_super(struct super_block *sb, void *data, int silent)struct inode *inode =NULL ;struct ocfs2_super *osb =NULL ;struct buffer_head *bh =NULL ; -char nodestr[8 ]; +char nodestr[12 ];We have the corresponding debugging logic at
ocfs2_dismount_volume(), which defines the nodestr with the old size, it seems that this change would cause potential buffer overflow while restore a bigger nodestr at it.
Yes, you are right. I will add the change in ocfs2_dismount_volume() as well. Sorry, I missed this.
Andrew: Would you prefer a combined patch or a separate one?
Quoted 6 lines
Also, there might has another relationship to ocfs2_control_message_setn/down at
stack_user.cbecause we limit the length of nodenum with a marco:
#defineOCFS2_CONTROL_MESSAGE_NODENUM_LEN8Maybe we should unify them?
This is a part of protocol to talk to the control daemon so we might not be able to just change this constant with changing other aspects such as the tools and the protocol versioning.
I am trying to get rid of control daemon handling altogether. Check my patches on "nocontrold". We are waiting for some review there. If that works, we don't need to change this constant ;)
Signature · 1 line
Goldwyn
PATCH 1/1 Use bigger nodestr to accomodate 32-bit node numbers
Quoted 9 lines · On Thu, 29 Aug 2013 07:15:00 -0500 Goldwyn Rodrigues <[email protected]> wrote:
Quoted 3 lines
We have the corresponding debugging logic at
ocfs2_dismount_volume(), which defines the nodestr with the old size, it seems that this change would cause potential buffer overflow while restore a bigger nodestr at it.Yes, you are right. I will add the change in
ocfs2_dismount_volume()as well. Sorry, I missed this.Andrew: Would you prefer a combined patch or a separate one?
It will need to be a separate patch, please.