Bug 1462969

Summary: Peer-file parsing is too fragile
Product: [Community] GlusterFS Reporter: Jeff Darcy <jeff>
Component: glusterdAssignee: bugs <bugs>
Status: CLOSED CURRENTRELEASE QA Contact:
Severity: unspecified Docs Contact:
Priority: unspecified    
Version: mainlineCC: amukherj, bugs
Target Milestone: ---Keywords: Triaged
Target Release: ---   
Hardware: Unspecified   
OS: Unspecified   
Whiteboard:
Fixed In Version: glusterfs-3.13.0 Doc Type: If docs needed, set a value
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2017-12-08 17:33:42 UTC Type: Bug
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:

Description Jeff Darcy 2017-06-19 18:54:17 UTC
Description of problem:

In glusterd_store_retrieve_peers we will open *any* file that we can find in $workdir/peers even if neither its name (which should look like a UUID) nor anything else about it suggests that it is in fact a peerfile.  If parsing it as a peerfile then fails, this causes the *entire* glusterd startup to fail.  Now, imagine what happens if some other kind of file happens to get dropped in that directory.  In my case it was a .gdb-history from debugging a zero-length-peer-file problem (no we don't even skip hidden files).  If you guessed that the mere presence of such an abomination caused startup to fail and debugging the original problem to be more difficult, give yourself a cookie.  Unless you wrote that code.  Then no cookie for you.

A better approach would be to check whether the file actually looks like a peerfile (i.e. the name is a UUID) before we even attempt to open it.  Then, if parsing still fails, just *skip it*.  The worst case is no worse than what happens now.  The more common case is that glusterd startup will succeed *and everything will be fine*, whereas now it just barfs in a non-obvious way.

Comment 1 Worker Ant 2017-07-26 17:42:14 UTC
REVIEW: https://review.gluster.org/17885 (glusterd: make peerfile parsing more robust) posted (#1) for review on master by Jeff Darcy (jeff.us)

Comment 2 Worker Ant 2017-07-26 18:08:11 UTC
REVIEW: https://review.gluster.org/17885 (glusterd: make peerfile parsing more robust) posted (#2) for review on master by Jeff Darcy (jeff.us)

Comment 3 Worker Ant 2017-07-26 19:00:33 UTC
REVIEW: https://review.gluster.org/17885 (glusterd: make peerfile parsing more robust) posted (#3) for review on master by Jeff Darcy (jeff.us)

Comment 4 Worker Ant 2017-07-27 04:55:27 UTC
COMMIT: https://review.gluster.org/17885 committed in master by Atin Mukherjee (amukherj) 
------
commit 16e8d03c7e4afa4c0e186f8ea50389fc3735e879
Author: Jeff Darcy <jdarcy>
Date:   Tue Jul 25 17:40:49 2017 -0700

    glusterd: make peerfile parsing more robust
    
    Differential Revision: https://phabricator.intern.facebook.com/D5498639
    
    Change-Id: I3184ed8f3dadbdcffd46f4ade855fa93131efa82
    BUG: 1462969
    Signed-off-by: Jeff Darcy <jdarcy>
    Reviewed-on: https://review.gluster.org/17885
    Smoke: Gluster Build System <jenkins.org>
    Tested-by: Jeff Darcy <jeff.us>
    Reviewed-by: Prashanth Pai <ppai>
    CentOS-regression: Gluster Build System <jenkins.org>
    Reviewed-by: Atin Mukherjee <amukherj>

Comment 5 Shyamsundar 2017-12-08 17:33:42 UTC
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.13.0, please open a new bug report.

glusterfs-3.13.0 has been announced on the Gluster mailinglists [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://lists.gluster.org/pipermail/announce/2017-December/000087.html
[2] https://www.gluster.org/pipermail/gluster-users/