New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

client/Client.cc: prevent segfaulting #12550

Merged
merged 1 commit into from Jan 19, 2017

Conversation

Projects
None yet
3 participants
@stiopaa1
Contributor

stiopaa1 commented Dec 17, 2016

The segfaulting in the rmdir function is caused by calling
filepath::last_dentry() function.
last_dentry() function assumes that the bits vector has always at
least one element, which is not the case for the the filepath object
created with "/" input.
This commit also fixes other functions affected by this bug:
link, unlink, rename, mkdir, mknod and symlink.

Fixes: http://tracker.ceph.com/issues/9935
Signed-off-by: Michal Jarzabek stiopa@gmail.com

@@ -6347,6 +6367,10 @@ int Client::mknod(const char *relpath, mode_t mode, const UserPerm& perms, dev_t
tout(cct) << relpath << std::endl;
tout(cct) << mode << std::endl;
tout(cct) << rdev << std::endl;
if(std::string(relpath) == "/")

This comment has been minimized.

@jcsp

jcsp Dec 21, 2016

Contributor

Minor style point: please could you make all these if statements have a space before the opening ( like this "if (std::string..."

This comment has been minimized.

@stiopaa1

stiopaa1 Dec 21, 2016

Contributor

Keep forgetting about this - will correct it!

r = may_create(dir.get(), perm);
}
{
filepath path(relpath);

This comment has been minimized.

@jcsp

jcsp Dec 21, 2016

Contributor

Please can you undo the move of the lower part into a {} block -- just adding the new condition above the "filepath path" initialization should be fine

This comment has been minimized.

@stiopaa1

stiopaa1 Dec 21, 2016

Contributor

If I undo this, the compiler will complain about goto skipping over definitions of variables.
I initially just wanted to do "return -EEXIST", but since the rest of the function is doing "goto out" I wanted to be consistent.

This comment has been minimized.

@jcsp

jcsp Jan 5, 2017

Contributor

It looks like the "goto out" stuff in this function was unnecessary anyway (nothing happening in out other than returning r), I would be happy for you to change all the "goto out"s into "return r" so that we don't need to use {} like this

client/Client.cc: prevent segfaulting
The segfaulting in the rmdir function is caused by calling
filepath::last_dentry() function.
last_dentry() function assumes that the bits vector has always at
least one element, which is not the case for the the filepath object
created with "/" input.
This commit also fixes other functions affected by this bug:
link, unlink, rename, mkdir, mknod and symlink.

Fixes: http://tracker.ceph.com/issues/9935
Signed-off-by: Michal Jarzabek <stiopa@gmail.com>
@stiopaa1

This comment has been minimized.

Contributor

stiopaa1 commented Jan 12, 2017

@jcsp
I have made the requested changes

@jcsp

This comment has been minimized.

Contributor

jcsp commented Jan 16, 2017

Looks good, will merge after next test run

@jcsp

jcsp approved these changes Jan 19, 2017

@jcsp jcsp merged commit 36874c8 into ceph:master Jan 19, 2017

2 of 3 checks passed

default Build finished.
Details
Signed-off-by all commits in this PR are signed
Details
Unmodifed Submodules submodules for project are unmodified
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment