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

src/msg/async/AsyncConnect.cc: Use of sizeof() on a Pointer Type #14773

Merged
merged 1 commit into from Apr 27, 2017

Conversation

Projects
None yet
5 participants
@SvyatoslavRazmyslov
Contributor

SvyatoslavRazmyslov commented Apr 25, 2017

We have found and fixed a weakness using PVS-Studio tool. PVS-Studio is a static code analyzer for C, C++ and C#: https://www.viva64.com/en/pvs-studio/

We suggests having a look at the emails, sent from @pvs-studio.com.

Analyzer warning: V568 It's odd that 'sizeof()' operator evaluates the size of a pointer to a class, but not the size of the 'seq' class object. AsyncConnection.cc 420

@yuyuyu101

This comment has been minimized.

Member

yuyuyu101 commented Apr 25, 2017

good catch, plz add signed-off-by with "git -s"

@SvyatoslavRazmyslov

This comment has been minimized.

Contributor

SvyatoslavRazmyslov commented Apr 25, 2017

@yuyuyu101

It works?

@yuyuyu101

This comment has been minimized.

Member

yuyuyu101 commented Apr 25, 2017

needs rebase

@tchaikov tchaikov added the bug fix label Apr 26, 2017

@tchaikov

This comment has been minimized.

Contributor

tchaikov commented Apr 26, 2017

@SvyatoslavRazmyslov could you just drop 1970ccb and keep 738e260 in your PR?

@SvyatoslavRazmyslov

This comment has been minimized.

Contributor

SvyatoslavRazmyslov commented Apr 26, 2017

@tchaikov

Please send me some correct commands for git.

@tchaikov

This comment has been minimized.

Contributor

tchaikov commented Apr 26, 2017

@SvyatoslavRazmyslov assuming the ceph/ceph repo is named "origin" in your workspace, and your own repo is named "viva64":

$ git remote -v
ci      git@github.com:ceph/ceph-ci.git (fetch)
ci      git@github.com:ceph/ceph-ci.git (push)
origin  git@github.com:ceph/ceph.git (fetch)
origin  git@github.com:ceph/ceph.git (push)
viva64  git@github.com:viva64/ceph.git (fetch)
viva64  git@github.com:viva64/ceph.git (push)
$ git checkout -b wip-14773 origin/master
Branch wip-14773 set up to track remote branch master from origin.
Switched to a new branch 'wip-14773'
$ git cherry-pick 738e260c26794401b48948dacdc983db8f6abfb1
[wip-14773 5b6b8e2b64] src/msg/async/AsyncConnect.cc: PVS-Studio: CWE-467 Use of sizeof() on a Pointer Type
 Author: Svyatoslav <razmyslov@viva64.com>
 Date: Tue Apr 25 15:52:53 2017 +0300
 1 file changed, 1 insertion(+), 1 deletion(-)
$ git push -f viva64 wip-14773:sizeof_pointer

let me know if anything else i can help.

src/msg/async/AsyncConnect.cc: PVS-Studio: CWE-467 Use of sizeof() on…
… a Pointer Type

Signed-off-by: Svyatoslav <razmyslov@viva64.com>

@tchaikov tchaikov added the needs-qa label Apr 26, 2017

@ghost

This comment has been minimized.

ghost commented Apr 26, 2017

@SvyatoslavRazmyslov Please stop spamming me with marketing mails for your proprietary product. I've received no less than 10 emails in the past few days. I will never buy nor recommend any kind of proprietary product. The more you try the more upset I'll be.

@liewegas liewegas changed the title from src/msg/async/AsyncConnect.cc: PVS-Studio: CWE-467 Use of sizeof() on a Pointer Type to src/msg/async/AsyncConnect.cc: Use of sizeof() on a Pointer Type Apr 26, 2017

@@ -417,7 +417,7 @@ void AsyncConnection::process()
case STATE_OPEN_TAG_ACK:
{
ceph_le64 *seq;
r = read_until(sizeof(seq), state_buffer);
r = read_until(sizeof(*seq), state_buffer);

This comment has been minimized.

@tchaikov

tchaikov Apr 26, 2017

Contributor

@yuyuyu101 do we need this fix in jewel?

This comment has been minimized.

@yuyuyu101

yuyuyu101 Apr 26, 2017

Member

... I'm not sure... sizeof(seq) == sizeof(*seq) in x86-64 arch. in current ceph supported archs, it should be no effect.

This comment has been minimized.

@tchaikov

tchaikov Apr 26, 2017

Contributor

yeah, maybe not... if we assume our user are all on LP64.

@yuriw yuriw merged commit a00005c into ceph:master Apr 27, 2017

4 checks passed

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