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

confile: pointer 'retv' is dereferenced. #2364

Closed
wants to merge 1 commit into from
Closed

confile: pointer 'retv' is dereferenced. #2364

wants to merge 1 commit into from

Conversation

2xsec
Copy link
Contributor

@2xsec 2xsec commented May 30, 2018

Hello,

If 'retv' pointer is null, snprintf can derefernce a null pointer.
snprintf allows null pointer with length = 0. In case of this, calling snprintf and other codes are inefficient.

Signed-off-by: Donghwa Jeong dh48.jeong@samsung.com

Signed-off-by: Donghwa Jeong <dh48.jeong@samsung.com>
@lxc-jenkins
Copy link

This pull request didn't trigger Jenkins as its author isn't in the whitelist.

An organization member must perform one of the following:

  • To have this branch tested by Jenkins, use the "ok to test" command.
  • To have a one time test done, use the "test this please" command.

Those commands are simple Github comments of the format: "jenkins: COMMAND"

@brauner
Copy link
Member

brauner commented May 30, 2018

jenkins: test this please

inlen = 0;
else
memset(retv, 0, inlen);
return -1;
Copy link
Member

Choose a reason for hiding this comment

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

The idea is that users can pass in NULL for retv in which case we report back the number of bytes that need to be allocated for the config item in question to be retrieved, i.e.:

	/*!
	 * \brief Retrieve the value of a config item.
	 *
	 * \param c Container.
	 * \param key Name of option to get.
	 * \param[out] retv Caller-allocated buffer to write value of \p key
	 * into (or \c NULL to determine length of value).
	 * \param inlen Length of \p retv (may be zero).
	 *
	 * \return Length of config items value, or < 0 on error.
	 *
	 * \note The caller can (and should) determine how large a buffer to allocate for
	 *  \p retv by initially passing its value as \c NULL and considering the return value.
	 *  This function can then be called again passing a newly-allocated suitably-sized buffer.
	 * \note If \p retv is NULL, \p inlen is ignored.
	 * \note If \p inlen is smaller than required, nothing will be written to \p retv and still return
	 *  the length of config item value.
	 */
	int (*get_config_item)(struct lxc_container *c, const char *key, char *retv, int inlen);

Your change would break this use-case. :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, you are right. However, As far as I know, C99 allows str to be null but SUSv2 is not.
Anyway, thanks for your review.

Copy link
Member

Choose a reason for hiding this comment

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

@2xsec, ah, I see what you're getting at here. Let me think for a sec.

Copy link
Member

Choose a reason for hiding this comment

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

Actually, SUSv2 specifies a return value less than 1 so it should be safe but we can improve this a little.

Copy link
Member

Choose a reason for hiding this comment

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

SUSv2 shouldn't be in use anymore but we can make it safe by passing in a non-null buffer but 0 as size.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK, I'll try to make it.

Copy link
Member

Choose a reason for hiding this comment

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

Oh, sorry I jumped the gun in #2365. :) I wait for your comment next time. :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's OK^^

@brauner brauner added the Incomplete Waiting on more information from reporter label May 30, 2018
@brauner
Copy link
Member

brauner commented May 30, 2018

Closed in favor of #2365. Thanks!

@brauner brauner closed this May 30, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Incomplete Waiting on more information from reporter
Development

Successfully merging this pull request may close these issues.

None yet

3 participants