Skip to content

Commit

Permalink
Fix double free in zfs_ioc_log_history()
Browse files Browse the repository at this point in the history
Under Linux tsd_set() will call the destructor on the thread
specific data when the passed value is NULL.  Therefore, there
is no need to call strfree() on the poolname after tsd_set().
The call to tsd_set() must also be moved after spa_open() to
prevent a use-after-free style defect.

Signed-off-by: Brian Behlendorf <[email protected]>
Issue openzfs#4872
  • Loading branch information
behlendorf committed Jul 26, 2016
1 parent 25458cb commit 2d713a6
Showing 1 changed file with 7 additions and 5 deletions.
12 changes: 7 additions & 5 deletions module/zfs/zfs_ioctl.c
Original file line number Diff line number Diff line change
Expand Up @@ -3340,14 +3340,16 @@ zfs_ioc_log_history(const char *unused, nvlist_t *innvl, nvlist_t *outnvl)
/*
* The poolname in the ioctl is not set, we get it from the TSD,
* which was set at the end of the last successful ioctl that allows
* logging. The secpolicy func already checked that it is set.
* Only one log ioctl is allowed after each successful ioctl, so
* we clear the TSD here.
* logging. The secpolicy func already checked that it is set but
* that assertion is reverified and handled. Only one log ioctl is
* allowed after each successful ioctl, so we clear the TSD here.
*/
poolname = tsd_get(zfs_allow_log_key);
(void) tsd_set(zfs_allow_log_key, NULL);
if (poolname == NULL)
return (SET_ERROR(EINVAL));

error = spa_open(poolname, &spa, FTAG);
strfree(poolname);
(void) tsd_set(zfs_allow_log_key, NULL);
if (error != 0)
return (error);

Expand Down

1 comment on commit 2d713a6

@heary-cao
Copy link

@heary-cao heary-cao commented on 2d713a6 Jul 27, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dear behlendorf , I am happy to see it. thinks for Answer it!

I agree with tsd_get, to determine whether poolname is NULL,
To ensure that the poolName spa_open is not NULL.
but, if isn't call to strfree, Will cause the memory leak poolName.
When to call tsd_set, set the entry he_value is NULL , therefore, when the call to strfree still is NULL. cause to coredump

I have a commit pull requests,
Signed: openzfs#4890

Proposed fix in openzfs#4890
reviews welcome.

Please sign in to comment.