Created attachment 898082 [details] memcpy change Description of problem: gf-history-changelog.c::gf_history_changelog_next_change() uses a memcpy to copy a null terminated string from buffer to another. The code also attempts to null terminate the destination buffer but actually modifies the source buffer. https://github.com/gluster/glusterfs/blame/master/xlators/features/changelog/lib/src/gf-history-changelog.c#L179 memcpy (bufptr, buffer, size - 1); *(buffer + size) = '\0'; This code can be replaced with a strncpy() Version-Release number of selected component (if applicable): 3.5 How reproducible: 100% Steps to Reproduce: 1. 2. 3. Actual results: Expected results: See attached patch Additional info:
REVIEW: http://review.gluster.org/7836 (features/changelog: NULL termination after memcpy should be of dest pointer) posted (#1) for review on master by Harshavardhana (harsha)
The patch is actually wrong - strncpy is unsafe and shouldn't be used. memcpy is still the right approach and faster implementation, just that it needs to be more cleaner by using the destination pointer to be NULL terminated.
REVIEW: http://review.gluster.org/7836 (features/changelog: NULL termination after memcpy should be of dest pointer) posted (#2) for review on master by Harshavardhana (harsha)
COMMIT: http://review.gluster.org/7836 committed in master by Venky Shankar (vshankar) ------ commit 38b2531d91e51dc73ba99ebcd3b98db75affa7d7 Author: Harshavardhana <harsha> Date: Wed May 21 13:30:02 2014 -0700 features/changelog: NULL termination after memcpy should be of dest pointer snippet code < ..memcpy (bufff, src, len - 1); ..*(src + len) = '\0'; ---> Wrong! > Source buffer lvalue() referencing with offset style NULL termination is wrong and unnecessary when we have a destination buffer, it is the destination buffer which should look to be NULL terminated Makes it more readable and also clearly logical. < ..memcpy (bufff, src, len - 1); ..bufff[len -1] = '\0'; ---> Correct! > Change-Id: I6d7f312aaa5c541f0345649ff1ef9f193892b674 BUG: 1099986 Signed-off-by: Harshavardhana <harsha> Reviewed-on: http://review.gluster.org/7836 Reviewed-by: Raghavendra Bhat <raghavendra> Reviewed-by: Venky Shankar <vshankar> Tested-by: Venky Shankar <vshankar>
A beta release for GlusterFS 3.6.0 has been released. Please verify if the release solves this bug report for you. In case the glusterfs-3.6.0beta1 release does not have a resolution for this issue, leave a comment in this bug and move the status to ASSIGNED. If this release fixes the problem for you, leave a note and change the status to VERIFIED. Packages for several distributions should become available in the near future. Keep an eye on the Gluster Users mailinglist [2] and the update (possibly an "updates-testing" repository) infrastructure for your distribution. [1] http://supercolony.gluster.org/pipermail/gluster-users/2014-September/018836.html [2] http://supercolony.gluster.org/pipermail/gluster-users/
This bug is getting closed because a release has been made available that should address the reported issue. In case the problem is still not fixed with glusterfs-3.6.1, please reopen this bug report. glusterfs-3.6.1 has been announced [1], packages for several distributions should become available in the near future. Keep an eye on the Gluster Users mailinglist [2] and the update infrastructure for your distribution. [1] http://supercolony.gluster.org/pipermail/gluster-users/2014-November/019410.html [2] http://supercolony.gluster.org/mailman/listinfo/gluster-users