This service will be undergoing maintenance at 00:00 UTC, 2017-10-23 It is expected to last about 30 minutes
Bug 1462969 - Peer-file parsing is too fragile
Peer-file parsing is too fragile
Status: MODIFIED
Product: GlusterFS
Classification: Community
Component: glusterd (Show other bugs)
mainline
Unspecified Unspecified
unspecified Severity unspecified
: ---
: ---
Assigned To: bugs@gluster.org
: Triaged
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2017-06-19 14:54 EDT by Jeff Darcy
Modified: 2017-08-08 11:45 EDT (History)
2 users (show)

See Also:
Fixed In Version:
Doc Type: If docs needed, set a value
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed:
Type: Bug
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)

  None (edit)
Description Jeff Darcy 2017-06-19 14:54:17 EDT
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 13:42:14 EDT
REVIEW: https://review.gluster.org/17885 (glusterd: make peerfile parsing more robust) posted (#1) for review on master by Jeff Darcy (jeff@pl.atyp.us)
Comment 2 Worker Ant 2017-07-26 14:08:11 EDT
REVIEW: https://review.gluster.org/17885 (glusterd: make peerfile parsing more robust) posted (#2) for review on master by Jeff Darcy (jeff@pl.atyp.us)
Comment 3 Worker Ant 2017-07-26 15:00:33 EDT
REVIEW: https://review.gluster.org/17885 (glusterd: make peerfile parsing more robust) posted (#3) for review on master by Jeff Darcy (jeff@pl.atyp.us)
Comment 4 Worker Ant 2017-07-27 00:55:27 EDT
COMMIT: https://review.gluster.org/17885 committed in master by Atin Mukherjee (amukherj@redhat.com) 
------
commit 16e8d03c7e4afa4c0e186f8ea50389fc3735e879
Author: Jeff Darcy <jdarcy@fb.com>
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@fb.com>
    Reviewed-on: https://review.gluster.org/17885
    Smoke: Gluster Build System <jenkins@build.gluster.org>
    Tested-by: Jeff Darcy <jeff@pl.atyp.us>
    Reviewed-by: Prashanth Pai <ppai@redhat.com>
    CentOS-regression: Gluster Build System <jenkins@build.gluster.org>
    Reviewed-by: Atin Mukherjee <amukherj@redhat.com>

Note You need to log in before you can comment on or make changes to this bug.