So o2dlm expects a null terminated domain name. The original patch is good as it adds the check in o2dlm only.
For userspace, we allow non-null terminating group (domain) name. That remains unchanged. On 07/12/2010 04:30 AM, Wengang Wang wrote: > Hi Dan, > > I think O2NM_MAX_NAME_LEN is the max valid length of the domain name. > Regarding your patch, it changed to be that a domain name with length > O2NM_MAX_NAME_LEN (NULL character not included) is not permitted. > > Though that check seems useless for current calls, we'd better keep it. > > Checking the structure, > > 99 struct ocfs2_cluster_connection { > 100 char cc_name[GROUP_NAME_MAX]; > 101 int cc_namelen; > > cc_name is not a NULL tailed string. the cc_namelen specifies the length of > it. > There does is misuse of cc_name, such as > > > 7 832 fs/ocfs2/stack_user.c<<user_cluster_connect>> > rc = dlm_new_lockspace(conn->cc_name, strlen(conn->cc_name), > > 5 308 fs/ocfs2/stack_o2cb.c<<o2cb_cluster_connect>> > dlm = dlm_register_domain(conn->cc_name, dlm_key,&fs_version); > > Also, the uuid shouldn't be treated as NULL tailed string. > 142 struct ocfs2_control_message_down { > 143 char tag[OCFS2_CONTROL_MESSAGE_OP_LEN]; > 144 char space1; > 145 char uuid[OCFS2_TEXT_UUID_LEN]; > 146 char space2; > > thus, > the calling of > fs/ocfs2/stack_user.c:474: ocfs2_control_send_down(msg->uuid, nodenum); > ->ocfs2_connection_find(uuid) > ->size_t len = strlen(name); > > is suspectable. > > Could you please make patch for that instead? > > regards, > wengang. > > On 10-07-10 16:33, Dan Carpenter wrote: > >> This function is only called from one place and it's like this: >> dlm_register_domain(conn->cc_name, dlm_key,&fs_version); >> >> The "conn->cc_name" is 64 characters long. If strlen(conn->cc_name) >> were equal to O2NM_MAX_NAME_LEN (64) that would be a bug because >> strlen() doesn't count the NULL character. >> >> In fact, if you look how O2NM_MAX_NAME_LEN is used, it mostly describes >> 64 character buffers. The only exception is nd_name from struct >> o2nm_node. >> >> Anyway I looked into it and in this case the domain string comes from >> osb->uuid_str in ocfs2_setup_osb_uuid(). That's 32 characters and NULL >> which easily fits into O2NM_MAX_NAME_LEN. This patch doesn't change how >> the code works, but I think it makes the code a little cleaner. >> >> Signed-off-by: Dan Carpenter<erro...@gmail.com> >> --- >> Or we could get rid of check entirely. >> >> diff --git a/fs/ocfs2/dlm/dlmdomain.c b/fs/ocfs2/dlm/dlmdomain.c >> index 6b5a492..084b051 100644 >> --- a/fs/ocfs2/dlm/dlmdomain.c >> +++ b/fs/ocfs2/dlm/dlmdomain.c >> @@ -1671,7 +1671,7 @@ struct dlm_ctxt * dlm_register_domain(const char >> *domain, >> struct dlm_ctxt *dlm = NULL; >> struct dlm_ctxt *new_ctxt = NULL; >> >> - if (strlen(domain)> O2NM_MAX_NAME_LEN) { >> + if (strlen(domain)>= O2NM_MAX_NAME_LEN) { >> ret = -ENAMETOOLONG; >> mlog(ML_ERROR, "domain name length too long\n"); >> goto leave; >> _______________________________________________ Ocfs2-devel mailing list Ocfs2-devel@oss.oracle.com http://oss.oracle.com/mailman/listinfo/ocfs2-devel