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 invalid memory read in kvs module #1065

Merged
merged 2 commits into from May 17, 2017

Conversation

Projects
None yet
4 participants
@garlick
Copy link
Member

garlick commented May 16, 2017

This PR addresses an invalid memory read in getroot_rpc() noted by @grondo in #1064.
It also fixes similar problematic code in setroot(), although it doesn't look like that one was actually causing a problem.

garlick added some commits May 16, 2017

modules/kvs: defend against invaid read in setroot
Problem: setroot() assumes that the length of the source
blobref is sizeof (href_t).

Change memcpy to a strcpy and add an assertion that
the source string is not too long, while allowing
source strings that are shorter.
@grondo

This comment has been minimized.

Copy link
Contributor

grondo commented May 16, 2017

Nice, thanks for taking a look at that. Looks good to me, and I'll merge assuming travis passes.

@coveralls

This comment has been minimized.

Copy link

coveralls commented May 17, 2017

Coverage Status

Coverage decreased (-0.1%) to 78.042% when pulling e51cc1c on garlick:kvs_memfix into 50fe8ab on flux-framework:master.

@codecov-io

This comment has been minimized.

Copy link

codecov-io commented May 17, 2017

Codecov Report

Merging #1065 into master will decrease coverage by 0.1%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #1065      +/-   ##
==========================================
- Coverage    77.9%   77.79%   -0.11%     
==========================================
  Files         148      148              
  Lines       25771    25772       +1     
==========================================
- Hits        20076    20049      -27     
- Misses       5695     5723      +28
Impacted Files Coverage Δ
src/modules/kvs/kvs.c 80.45% <100%> (-0.1%) ⬇️
src/broker/content-cache.c 72.54% <0%> (-1.75%) ⬇️
src/broker/module.c 82.15% <0%> (-1.7%) ⬇️
src/modules/connector-local/local.c 70.49% <0%> (-1.23%) ⬇️
src/cmd/flux-event.c 65.55% <0%> (-1.12%) ⬇️
src/common/libflux/rpc.c 92% <0%> (-1.1%) ⬇️
src/broker/overlay.c 71.32% <0%> (-0.7%) ⬇️
src/common/libflux/handle.c 85.07% <0%> (-0.57%) ⬇️
src/common/libutil/veb.c 98.31% <0%> (-0.57%) ⬇️
... and 5 more

@grondo grondo merged commit 61bae10 into flux-framework:master May 17, 2017

4 checks passed

codecov/patch 100% of diff hit (target 77.9%)
Details
codecov/project Absolute coverage decreased by -0.1% but relative coverage increased by +22.09% compared to 50fe8ab
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage decreased (-0.1%) to 78.042%
Details

@grondo grondo referenced this pull request Aug 23, 2017

Closed

0.8.0 Release #1160

@garlick garlick deleted the garlick:kvs_memfix branch Sep 6, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.