Skip to content
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

Fix: Call zfs_get_name with invalid parameter #4919

Closed
wants to merge 1 commit into from

Conversation

GeLiXin
Copy link
Contributor

@GeLiXin GeLiXin commented Aug 2, 2016

Dear all:
zfs_get_name() expect parameter of type zfs_handle_t *zhp , but get an invalid parameter type of zfs_handle_t **zhp actually in libzfs_dataset_cmp(), which may trigger a coredump if called.

libzfs_dataset_cmp() working normally so far, just because all the callers only give datasets whoes type is ZFS_TYPE_FILESYSTEM to it now, we compared their mountpoint and return, luckily.

It's a risk when the caller give snapshot or volume as input parameter.

Signed-off-by: GeLiXin ge.lixin@zte.com.cn

@GeLiXin GeLiXin changed the title Fix:call zfs_get_name with invalid parameter Fix: Call zfs_get_name with invalid parameter Aug 2, 2016
@behlendorf
Copy link
Contributor

LGTM. Nice find, @dweeezil can you take a look at this too.

@behlendorf behlendorf added this to the 0.7.0 milestone Aug 2, 2016
@dweeezil
Copy link
Contributor

dweeezil commented Aug 2, 2016

At first glance, this appears to be completely correct but I'm mobile now so I'll look at it again tomorrow.

@dweeezil
Copy link
Contributor

dweeezil commented Aug 4, 2016

Yes, this fix is of course completely correct and should be pulled into other OpenZFS implementations as well.

On a somewhat related notes, there's a bit of unnecessary void pointer munging related to the cb_handles array. There's no good reason that proper typecasts couldn't be used, for example, in libzfs_add_handle() but, of course, the fix would be mostly cosmetic.

@GeLiXin
Copy link
Contributor Author

GeLiXin commented Aug 4, 2016

@dweeezil Thanks for your review. I agree with you on that the void pointer munging related to the cb_handles array which can't be used direct is unnecessary. On the other hand, I guess this typecasts is on purpose of making the code easy to be understood, so I just leave it stay the same in this commit.

If you think this typecasts really need to be improved, I can make a further fix.

@behlendorf behlendorf closed this in 88c4c7a Aug 8, 2016
@behlendorf
Copy link
Contributor

Merged, this is one which should be pushed upstream. We can address the typecasts in a follow up patch if we choose too.

88c4c7a Fix call zfs_get_name() with invalid parameter

nedbass pushed a commit to nedbass/zfs that referenced this pull request Sep 3, 2016
zfs_get_name() expects a parameter of type zfs_handle_t *zhp , but
gets an invalid parameter type of zfs_handle_t **zhp actually in
libzfs_dataset_cmp(), which may trigger a coredump if called.

libzfs_dataset_cmp() working normally so far, just because all the
callers only give datasets of type ZFS_TYPE_FILESYSTEM to it, we
compared their mountpoint and return, luckily.

Signed-off-by: GeLiXin <ge.lixin@zte.com.cn>
Signed-off-by: Tim Chase <tim@chase2k.com>
Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov>
Closes openzfs#4919
nedbass pushed a commit to nedbass/zfs that referenced this pull request Sep 5, 2016
zfs_get_name() expects a parameter of type zfs_handle_t *zhp , but
gets an invalid parameter type of zfs_handle_t **zhp actually in
libzfs_dataset_cmp(), which may trigger a coredump if called.

libzfs_dataset_cmp() working normally so far, just because all the
callers only give datasets of type ZFS_TYPE_FILESYSTEM to it, we
compared their mountpoint and return, luckily.

Signed-off-by: GeLiXin <ge.lixin@zte.com.cn>
Signed-off-by: Tim Chase <tim@chase2k.com>
Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov>
Closes openzfs#4919
nedbass pushed a commit to nedbass/zfs that referenced this pull request Sep 5, 2016
zfs_get_name() expects a parameter of type zfs_handle_t *zhp , but
gets an invalid parameter type of zfs_handle_t **zhp actually in
libzfs_dataset_cmp(), which may trigger a coredump if called.

libzfs_dataset_cmp() working normally so far, just because all the
callers only give datasets of type ZFS_TYPE_FILESYSTEM to it, we
compared their mountpoint and return, luckily.

Signed-off-by: GeLiXin <ge.lixin@zte.com.cn>
Signed-off-by: Tim Chase <tim@chase2k.com>
Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov>
Closes openzfs#4919
tuxoko pushed a commit to tuxoko/zfs that referenced this pull request Sep 8, 2016
zfs_get_name() expects a parameter of type zfs_handle_t *zhp , but
gets an invalid parameter type of zfs_handle_t **zhp actually in
libzfs_dataset_cmp(), which may trigger a coredump if called.

libzfs_dataset_cmp() working normally so far, just because all the
callers only give datasets of type ZFS_TYPE_FILESYSTEM to it, we
compared their mountpoint and return, luckily.

Signed-off-by: GeLiXin <ge.lixin@zte.com.cn>
Signed-off-by: Tim Chase <tim@chase2k.com>
Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov>
Closes openzfs#4919
nedbass pushed a commit to nedbass/zfs that referenced this pull request Sep 9, 2016
zfs_get_name() expects a parameter of type zfs_handle_t *zhp , but
gets an invalid parameter type of zfs_handle_t **zhp actually in
libzfs_dataset_cmp(), which may trigger a coredump if called.

libzfs_dataset_cmp() working normally so far, just because all the
callers only give datasets of type ZFS_TYPE_FILESYSTEM to it, we
compared their mountpoint and return, luckily.

Signed-off-by: GeLiXin <ge.lixin@zte.com.cn>
Signed-off-by: Tim Chase <tim@chase2k.com>
Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov>
Closes openzfs#4919
nedbass pushed a commit to nedbass/zfs that referenced this pull request Sep 9, 2016
zfs_get_name() expects a parameter of type zfs_handle_t *zhp , but
gets an invalid parameter type of zfs_handle_t **zhp actually in
libzfs_dataset_cmp(), which may trigger a coredump if called.

libzfs_dataset_cmp() working normally so far, just because all the
callers only give datasets of type ZFS_TYPE_FILESYSTEM to it, we
compared their mountpoint and return, luckily.

Signed-off-by: GeLiXin <ge.lixin@zte.com.cn>
Signed-off-by: Tim Chase <tim@chase2k.com>
Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov>
Closes openzfs#4919
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants