Date: Wed, 30 May 2012 00:27:12 +0000 (UTC) From: "Justin T. Gibbs" <gibbs@FreeBSD.org> To: src-committers@freebsd.org, svn-src-projects@freebsd.org Subject: svn commit: r236268 - projects/zfsd/head/cddl/sbin/zfsd Message-ID: <201205300027.q4U0RCJN013862@svn.freebsd.org>
next in thread | raw e-mail | index | archive | help
Author: gibbs Date: Wed May 30 00:27:12 2012 New Revision: 236268 URL: http://svn.freebsd.org/changeset/base/236268 Log: Properly terminate truncated events with a newline (end of event token), and discard any data that exceeds zfsd's MAX_EVENT_SIZE limit until zfsd is resynchronized with the event stream. This resolves issues (de)serializing CaseFiles with truncated events. cddl/sbin/zfsd/case_file.cc: Do not assume that deserialization of all events from a case file will be successful. Just skip over unparsable events. cddl/sbin/zfsd/zfsd.cc: cddl/sbin/zfsd/zfsd.h: o In EventBuffer::ExtractEvent(), remove code and a comment dealing with "start tokens" and "end tokens" sharing characters. In the normal case of untruncated events, the event terminator is present and distinct from the start token. Devd immediately closes our unix domain socket if it cannot write a complete event, which means a start token for a new event after a truncted event cannot happen. o In EventBuffer::ExtractEvent(), remove dead code that tested for end of event tokens other than '\n'. Due to the way that strcspn() operates, this cannot happen. o Add stateful resynchronization to the EventBuffer class. EventBuffer can get out of synch whenever it truncates an incoming event due to its internal event size limit. The original code dealt with this issue by looking for start tokens. Since start tokens may also be valid within an event body, the only fully safe option is to discard data until the end of event token (which can't occur within an event) is read. o When truncating an event, properly terminate it so that when EventBuffer is used for CaseFile deserialization the input data is properly formed. o EventBuffer::UnParsed() and EventBuffer::NextEventMaxLen() are const. Enforce this via the const keyword. Sponsored by: Spectra Logic Corporation Modified: projects/zfsd/head/cddl/sbin/zfsd/case_file.cc projects/zfsd/head/cddl/sbin/zfsd/zfsd.cc projects/zfsd/head/cddl/sbin/zfsd/zfsd.h Modified: projects/zfsd/head/cddl/sbin/zfsd/case_file.cc ============================================================================== --- projects/zfsd/head/cddl/sbin/zfsd/case_file.cc Wed May 30 00:14:13 2012 (r236267) +++ projects/zfsd/head/cddl/sbin/zfsd/case_file.cc Wed May 30 00:27:12 2012 (r236268) @@ -531,7 +531,8 @@ CaseFile::DeSerializeFile(const char *fi EventBuffer eventBuffer(fd); while (eventBuffer.ExtractEvent(evString)) { DevCtlEvent *event(DevCtlEvent::CreateEvent(evString)); - caseFile->m_events.push_back(event); + if (event != NULL) + caseFile->m_events.push_back(event); } close(fd); } catch (const ParseException &exp) { Modified: projects/zfsd/head/cddl/sbin/zfsd/zfsd.cc ============================================================================== --- projects/zfsd/head/cddl/sbin/zfsd/zfsd.cc Wed May 30 00:14:13 2012 (r236267) +++ projects/zfsd/head/cddl/sbin/zfsd/zfsd.cc Wed May 30 00:27:12 2012 (r236268) @@ -108,7 +108,8 @@ EventBuffer::EventBuffer(int fd) : m_fd(fd), m_validLen(0), m_parsedLen(0), - m_nextEventOffset(0) + m_nextEventOffset(0), + m_synchronized(true) { } @@ -127,24 +128,18 @@ EventBuffer::ExtractEvent(string &eventS continue; } - char *nextEvent(m_buf + m_nextEventOffset); - size_t startLen(strcspn(nextEvent, s_eventStartTokens)); - bool aligned(startLen == 0); - if (aligned == false) { - syslog(LOG_WARNING, - "Re-synchronizing with devd event stream"); - m_nextEventOffset += startLen; + char *nextEvent(m_buf + m_nextEventOffset); + bool truncated(true); + size_t eventLen(strcspn(nextEvent, s_eventEndTokens)); + + if (!m_synchronized) { + /* Discard data until an end token is read. */ + if (nextEvent[eventLen] != '\0') + m_synchronized = true; + m_nextEventOffset += eventLen; m_parsedLen = m_nextEventOffset; continue; - } - - /* - * Start tokens may be end tokens too, so skip the start - * token when trying to find the end of the event. - */ - bool truncated(true); - size_t eventLen(strcspn(nextEvent + 1, s_eventEndTokens) + 1); - if (nextEvent[eventLen] == '\0') { + } else if (nextEvent[eventLen] == '\0') { m_parsedLen += eventLen; if (m_parsedLen < MAX_EVENT_SIZE) { @@ -156,9 +151,6 @@ EventBuffer::ExtractEvent(string &eventS } syslog(LOG_WARNING, "Event exceeds event size limit of %d bytes."); - } else if (nextEvent[eventLen] != '\n') { - syslog(LOG_WARNING, - "Improperly terminated event encountered."); } else { /* * Include the normal terminator in the extracted @@ -175,8 +167,13 @@ EventBuffer::ExtractEvent(string &eventS if (truncated) { size_t fieldEnd; + /* Break cleanly at the end of a key<=>value pair. */ fieldEnd = eventString.find_last_of(s_keyPairSepTokens); - eventString.erase(fieldEnd); + if (fieldEnd != string::npos) + eventString.erase(fieldEnd); + eventString += '\n'; + + m_synchronized = false; syslog(LOG_WARNING, "Truncated %d characters from event.", eventLen - fieldEnd); Modified: projects/zfsd/head/cddl/sbin/zfsd/zfsd.h ============================================================================== --- projects/zfsd/head/cddl/sbin/zfsd/zfsd.h Wed May 30 00:14:13 2012 (r236267) +++ projects/zfsd/head/cddl/sbin/zfsd/zfsd.h Wed May 30 00:27:12 2012 (r236268) @@ -143,10 +143,10 @@ private: }; /** The amount of data in m_buf we have yet to look at. */ - size_t UnParsed(); + size_t UnParsed() const; /** The amount of data in m_buf available for the next event. */ - size_t NextEventMaxLen(); + size_t NextEventMaxLen() const; /** Fill the event buffer with event data from Devd. */ bool Fill(); @@ -174,17 +174,20 @@ private: /** Offset to the start token of the next event. */ size_t m_nextEventOffset; + + /** The EventBuffer is aligned and tracking event records. */ + bool m_synchronized; }; //- EventBuffer Inline Private Methods ----------------------------------------- inline size_t -EventBuffer::UnParsed() +EventBuffer::UnParsed() const { return (m_validLen - m_parsedLen); } inline size_t -EventBuffer::NextEventMaxLen() +EventBuffer::NextEventMaxLen() const { return (m_validLen - m_nextEventOffset); }
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201205300027.q4U0RCJN013862>