25 Jul, 2013

1 commit

  • Several call sites use the hardcoded following condition :

    sk_stream_wspace(sk) >= sk_stream_min_wspace(sk)

    Lets use a helper because TCP_NOTSENT_LOWAT support will change this
    condition for TCP sockets.

    Signed-off-by: Eric Dumazet
    Cc: Neal Cardwell
    Cc: Yuchung Cheng
    Acked-by: Neal Cardwell
    Signed-off-by: David S. Miller

    Eric Dumazet
     

04 Jul, 2013

6 commits

  • We can't use !req->r_sent to check if OSD request is sent for the
    first time, this is because __cancel_request() zeros req->r_sent
    when OSD map changes. Rather than adding a new variable to struct
    ceph_osd_request to indicate if it's sent for the first time, We
    can call the unsafe callback only when unsafe OSD reply is received.
    If OSD's first reply is safe, just skip calling the unsafe callback.

    The purpose of unsafe callback is adding unsafe request to a list,
    so that fsync(2) can wait for the safe reply. fsync(2) doesn't need
    to wait for a write(2) that hasn't returned yet. So it's OK to add
    request to the unsafe list when the first OSD reply is received.
    (ceph_sync_write() returns after receiving the first OSD reply)

    Signed-off-by: Yan, Zheng
    Reviewed-by: Sage Weil

    Yan, Zheng
     
  • A malicious monitor can craft an auth reply message that could cause a
    NULL function pointer dereference in the client's kernel.

    To prevent this, the auth_none protocol handler needs an empty
    ceph_auth_client_ops->build_request() function.

    CVE-2013-1059

    Signed-off-by: Tyler Hicks
    Reported-by: Chanam Park
    Reviewed-by: Seth Arnold
    Reviewed-by: Sage Weil
    Cc: stable@vger.kernel.org

    Tyler Hicks
     
  • check the "not truncated yet" case

    Signed-off-by: Yan, Zheng
    Reviewed-by: Sage Weil

    Yan, Zheng
     
  • handle_reply() calls complete_request() only if the first OSD reply
    has ONDISK flag.

    Signed-off-by: Yan, Zheng
    Reviewed-by: Sage Weil

    Yan, Zheng
     
  • If an osd client response message arrives that has a front section
    that's too big for the buffer set aside to receive it, a warning
    gets reported and a new buffer is allocated.

    The warning says nothing about which connection had the problem.
    Add the peer type and number to what gets reported, to be a bit more
    informative.

    Signed-off-by: Alex Elder
    Reviewed-by: Josh Durgin

    Alex Elder
     
  • When an osd request is set to linger, the osd client holds onto the
    request so it can be re-submitted following certain osd map changes.
    The osd client holds a reference to the request until it is
    unregistered. This is used by rbd for watch requests.

    Currently, the reference is taken when the request is marked with
    the linger flag. This means that if an error occurs after that
    time but before the the request completes successfully, that
    reference is leaked.

    There's really no reason to take the reference until the request is
    registered in the the osd client's list of lingering requests, and
    that only happens when the lingering (watch) request completes
    successfully.

    So take that reference only when it gets registered following
    succesful completion, and drop it (as before) when the request
    gets unregistered. This avoids the reference problem on error
    in rbd.

    Rearrange ceph_osdc_unregister_linger_request() to avoid using
    the request pointer after it may have been freed.

    And hold an extra reference in kick_requests() while handling
    a linger request that has not yet been registered, to ensure
    it doesn't go away.

    This resolves:
    http://tracker.ceph.com/issues/3859

    Signed-off-by: Alex Elder
    Reviewed-by: Josh Durgin

    Alex Elder
     

18 May, 2013

1 commit

  • An osd client has a red-black tree describing its osds, and
    occasionally we would get crashes due to one of these trees tree
    becoming corrupt somehow.

    The problem turned out to be that reset_changed_osds() was being
    called without protection of the osd client request mutex. That
    function would call __reset_osd() for any osd that had changed, and
    __reset_osd() would call __remove_osd() for any osd with no
    outstanding requests, and finally __remove_osd() would remove the
    corresponding entry from the red-black tree. Thus, the tree was
    getting modified without having any lock protection, and was
    vulnerable to problems due to concurrent updates.

    This appears to be the only osd tree updating path that has this
    problem. It can be fairly easily fixed by moving the call up
    a few lines, to just before the request mutex gets dropped
    in kick_requests().

    This resolves:
    http://tracker.ceph.com/issues/5043

    Cc: stable@vger.kernel.org # 3.4+
    Signed-off-by: Alex Elder
    Reviewed-by: Sage Weil

    Alex Elder
     

14 May, 2013

1 commit

  • The rbd code has a need to be able to restart an osd request that
    has already been started and completed once before. This currently
    wouldn't work right because the osd client code assumes an osd
    request will be started exactly once Certain fields in a request
    are never cleared and this leads to trouble if you try to reuse it.

    Specifically, the r_sent, r_got_reply, and r_completed fields are
    never cleared. The r_sent field records the osd incarnation at the
    time the request was sent to that osd. If that's non-zero, the
    message won't get re-mapped to a target osd properly, and won't be
    put on the unsafe requests list the first time it's sent as it
    should. The r_got_reply field is used in handle_reply() to ensure
    the reply to a request is processed only once. And the r_completed
    field is used for lingering requests to avoid calling the callback
    function every time the osd client re-sends the request on behalf of
    its initiator.

    Each osd request passes through ceph_osdc_start_request() when
    responsibility for the request is handed over to the osd client for
    completion. We can safely zero these three fields there each time a
    request gets started.

    One last related change--clear the r_linger flag when a request
    is no longer registered as a linger request.

    This resolves:
    http://tracker.ceph.com/issues/5026

    Signed-off-by: Alex Elder
    Reviewed-by: Josh Durgin

    Alex Elder
     

03 May, 2013

3 commits


02 May, 2013

28 commits

  • This creates a new source file "net/ceph/snapshot.c" to contain
    utility routines related to ceph snapshot contexts. The main
    motivation was to define ceph_create_snap_context() as a common way
    to create these structures, but I've moved the definitions of
    ceph_get_snap_context() and ceph_put_snap_context() there too.
    (The benefit of inlining those is very small, and I'd rather
    keep this collection of functions together.)

    Signed-off-by: Alex Elder
    Reviewed-by: Josh Durgin

    Alex Elder
     
  • A WATCH op includes an object version. The version that's supplied
    is incorrectly byte-swapped osd_req_op_watch_init() where it's first
    assigned (it's been this way since that code was first added).

    The result is that the version sent to the osd is wrong, because
    that value gets byte-swapped again in osd_req_encode_op(). This
    is the source of a sparse warning related to improper byte order in
    the assignment.

    The approach of using the version to avoid a race is deprecated
    (see http://tracker.ceph.com/issues/3871), and the watch parameter
    is no longer even examined by the osd. So fix the assignment in
    osd_req_op_watch_init() so it no longer does the byte swap.

    This resolves:
    http://tracker.ceph.com/issues/3847

    Signed-off-by: Alex Elder
    Reviewed-by: Josh Durgin

    Alex Elder
     
  • Add the ability to provide an array of pages as outbound request
    data for object class method calls.

    Signed-off-by: Alex Elder
    Reviewed-by: Josh Durgin

    Alex Elder
     
  • This patch makes four small changes in the ceph messenger.

    While getting copyup functionality working I found two bugs in the
    messenger. Existing paths through the code did not trigger these
    problems, but they're fixed here:
    - In ceph_msg_data_pagelist_cursor_init(), the cursor's
    last_piece field was being checked against the length
    supplied. This was OK until this commit: ccba6d98 libceph:
    implement multiple data items in a message That commit changed
    the cursor init routines to allow lengths to be supplied that
    exceeded the size of the current data item. Because of this,
    we have to use the assigned cursor resid field rather than the
    provided length in determining whether the cursor points to
    the last piece of a data item.
    - In ceph_msg_data_add_pages(), a BUG_ON() was erroneously
    catching attempts to add page data to a message if the message
    already had data assigned to it. That was OK until that same
    commit, at which point it was fine for messages to have
    multiple data items. It slipped through because that BUG_ON()
    call was present twice in that function. (You can never be too
    careful.)

    In addition two other minor things are changed:
    - In ceph_msg_data_cursor_init(), the local variable "data" was
    getting assigned twice.
    - In ceph_msg_data_advance(), it was assumed that the
    type-specific advance routine would set new_piece to true
    after it advanced past the last piece. That may have been
    fine, but since we check for that case we might as well set it
    explicitly in ceph_msg_data_advance().

    This resolves:
    http://tracker.ceph.com/issues/4762

    Signed-off-by: Alex Elder
    Reviewed-by: Josh Durgin

    Alex Elder
     
  • Allow osd request ops that aren't otherwise structured (not class,
    extent, or watch ops) to specify "raw" data to be used to hold
    incoming data for the op. Make use of this capability for the osd
    STAT op.

    Prefix the name of the private function osd_req_op_init() with "_",
    and expose a new function by that (earlier) name whose purpose is to
    initialize osd ops with (only) implied data.

    For now we'll just support the use of a page array for an osd op
    with incoming raw data.

    Signed-off-by: Alex Elder
    Reviewed-by: Josh Durgin

    Alex Elder
     
  • There are a bunch of functions defined to encapsulate getting the
    address of a data field for a particular op in an osd request.
    They're all defined the same way, so create a macro to take the
    place of all of them.

    Two of these are used outside the osd client code, so preserve them
    (but convert them to use the new macro internally). Stop exporting
    the ones that aren't used elsewhere.

    Signed-off-by: Alex Elder
    Reviewed-by: Josh Durgin

    Alex Elder
     
  • In the incremental move toward supporting distinct data items in an
    osd request some of the functions had "write_request" parameters to
    indicate, basically, whether the data belonged to in_data or the
    out_data. Now that we maintain the data fields in the op structure
    there is no need to indicate the direction, so get rid of the
    "write_request" parameters.

    Signed-off-by: Alex Elder
    Reviewed-by: Josh Durgin

    Alex Elder
     
  • An osd request currently has two callbacks. They inform the
    initiator of the request when we've received confirmation for the
    target osd that a request was received, and when the osd indicates
    all changes described by the request are durable.

    The only time the second callback is used is in the ceph file system
    for a synchronous write. There's a race that makes some handling of
    this case unsafe. This patch addresses this problem. The error
    handling for this callback is also kind of gross, and this patch
    changes that as well.

    In ceph_sync_write(), if a safe callback is requested we want to add
    the request on the ceph inode's unsafe items list. Because items on
    this list must have their tid set (by ceph_osd_start_request()), the
    request added *after* the call to that function returns. The
    problem with this is that there's a race between starting the
    request and adding it to the unsafe items list; the request may
    already be complete before ceph_sync_write() even begins to put it
    on the list.

    To address this, we change the way the "safe" callback is used.
    Rather than just calling it when the request is "safe", we use it to
    notify the initiator the bounds (start and end) of the period during
    which the request is *unsafe*. So the initiator gets notified just
    before the request gets sent to the osd (when it is "unsafe"), and
    again when it's known the results are durable (it's no longer
    unsafe). The first call will get made in __send_request(), just
    before the request message gets sent to the messenger for the first
    time. That function is only called by __send_queued(), which is
    always called with the osd client's request mutex held.

    We then have this callback function insert the request on the ceph
    inode's unsafe list when we're told the request is unsafe. This
    will avoid the race because this call will be made under protection
    of the osd client's request mutex. It also nicely groups the setup
    and cleanup of the state associated with managing unsafe requests.

    The name of the "safe" callback field is changed to "unsafe" to
    better reflect its new purpose. It has a Boolean "unsafe" parameter
    to indicate whether the request is becoming unsafe or is now safe.
    Because the "msg" parameter wasn't used, we drop that.

    This resolves the original problem reportedin:
    http://tracker.ceph.com/issues/4706

    Reported-by: Yan, Zheng
    Signed-off-by: Alex Elder
    Reviewed-by: Yan, Zheng
    Reviewed-by: Sage Weil

    Alex Elder
     
  • Right now the data for a method call is specified via a pointer and
    length, and it's copied--along with the class and method name--into
    a pagelist data item to be sent to the osd. Instead, encode the
    data in a data item separate from the class and method names.

    This will allow large amounts of data to be supplied to methods
    without copying. Only rbd uses the class functionality right now,
    and when it really needs this it will probably need to use a page
    array rather than a page list. But this simple implementation
    demonstrates the functionality on the osd client, and that's enough
    for now.

    This resolves:
    http://tracker.ceph.com/issues/4104

    Signed-off-by: Alex Elder
    Reviewed-by: Josh Durgin

    Alex Elder
     
  • Change the names of the functions that put data on a pagelist to
    reflect that we're adding to whatever's already there rather than
    just setting it to the one thing. Currently only one data item is
    ever added to a message, but that's about to change.

    This resolves:
    http://tracker.ceph.com/issues/2770

    Signed-off-by: Alex Elder
    Reviewed-by: Josh Durgin

    Alex Elder
     
  • This patch adds support to the messenger for more than one data item
    in its data list.

    A message data cursor has two more fields to support this:
    - a count of the number of bytes left to be consumed across
    all data items in the list, "total_resid"
    - a pointer to the head of the list (for validation only)

    The cursor initialization routine has been split into two parts: the
    outer one, which initializes the cursor for traversing the entire
    list of data items; and the inner one, which initializes the cursor
    to start processing a single data item.

    When a message cursor is first initialized, the outer initialization
    routine sets total_resid to the length provided. The data pointer
    is initialized to the first data item on the list. From there, the
    inner initialization routine finishes by setting up to process the
    data item the cursor points to.

    Advancing the cursor consumes bytes in total_resid. If the resid
    field reaches zero, it means the current data item is fully
    consumed. If total_resid indicates there is more data, the cursor
    is advanced to point to the next data item, and then the inner
    initialization routine prepares for using that. (A check is made at
    this point to make sure we don't wrap around the front of the list.)

    The type-specific init routines are modified so they can be given a
    length that's larger than what the data item can support. The resid
    field is initialized to the smaller of the provided length and the
    length of the entire data item.

    When total_resid reaches zero, we're done.

    This resolves:
    http://tracker.ceph.com/issues/3761

    Signed-off-by: Alex Elder
    Reviewed-by: Josh Durgin

    Alex Elder
     
  • In place of the message data pointer, use a list head which links
    through message data items. For now we only support a single entry
    on that list.

    Signed-off-by: Alex Elder
    Reviewed-by: Josh Durgin

    Alex Elder
     
  • Rather than having a ceph message data item point to the cursor it's
    associated with, have the cursor point to a data item. This will
    allow a message cursor to be used for more than one data item.

    Signed-off-by: Alex Elder
    Reviewed-by: Josh Durgin

    Alex Elder
     
  • A message will only be processing a single data item at a time, so
    there's no need for each data item to have its own cursor.

    Move the cursor embedded in the message data structure into the
    message itself. To minimize the impact, keep the data->cursor
    field, but make it be a pointer to the cursor in the message.

    Move the definition of ceph_msg_data above ceph_msg_data_cursor so
    the cursor can point to the data without a forward definition rather
    than vice-versa.

    This and the upcoming patches are part of:
    http://tracker.ceph.com/issues/3761

    Signed-off-by: Alex Elder
    Reviewed-by: Josh Durgin

    Alex Elder
     
  • The bio is the only data item type that doesn't record its full
    length. Fix that.

    Signed-off-by: Alex Elder
    Reviewed-by: Josh Durgin

    Alex Elder
     
  • We know the length of our message buffers. If we get a message
    that's too long, just dump it and ignore it. If skip was set
    then con->in_msg won't be valid, so be careful not to dereference
    a null pointer in the process.

    This resolves:
    http://tracker.ceph.com/issues/4664

    Signed-off-by: Alex Elder
    Reviewed-by: Josh Durgin

    Alex Elder
     
  • This patch:
    15a0d7b libceph: record message data length
    did not enclose some bio-specific code inside CONFIG_BLOCK as
    it should have. Fix that.

    Signed-off-by: Alex Elder
    Reviewed-by: Josh Durgin

    Alex Elder
     
  • Finally! Convert the osd op data pointers into real structures, and
    make the switch over to using them instead of having all ops share
    the in and/or out data structures in the osd request.

    Set up a new function to traverse the set of ops and release any
    data associated with them (pages).

    This and the patches leading up to it resolve:
    http://tracker.ceph.com/issues/4657

    Signed-off-by: Alex Elder
    Reviewed-by: Josh Durgin

    Alex Elder
     
  • Still using the osd request r_data_in and r_data_out pointer, but
    we're basically only referring to it via the data pointers in the
    osd ops. And we're transferring that information to the request
    or reply message only when the op indicates it's needed, in
    osd_req_encode_op().

    To avoid a forward reference, ceph_osdc_msg_data_set() was moved up
    in the file.

    Don't bother calling ceph_osd_data_init(), in ceph_osd_alloc(),
    because the ops array will already be zeroed anyway.

    Signed-off-by: Alex Elder
    Reviewed-by: Josh Durgin

    Alex Elder
     
  • This ends up being a rather large patch but what it's doing is
    somewhat straightforward.

    Basically, this is replacing two calls with one. The first of the
    two calls is initializing a struct ceph_osd_data with data (either a
    page array, a page list, or a bio list); the second is setting an
    osd request op so it associates that data with one of the op's
    parameters. In place of those two will be a single function that
    initializes the op directly.

    That means we sort of fan out a set of the needed functions:
    - extent ops with pages data
    - extent ops with pagelist data
    - extent ops with bio list data
    and
    - class ops with page data for receiving a response

    We also have define another one, but it's only used internally:
    - class ops with pagelist data for request parameters

    Note that we *still* haven't gotten rid of the osd request's
    r_data_in and r_data_out fields. All the osd ops refer to them for
    their data. For now, these data fields are pointers assigned to the
    appropriate r_data_* field when these new functions are called.

    Signed-off-by: Alex Elder
    Reviewed-by: Josh Durgin

    Alex Elder
     
  • All calls of ceph_osdc_start_request() are preceded (in the case of
    rbd, almost) immediately by a call to ceph_osdc_build_request().

    Move the build calls at the top of ceph_osdc_start_request() out of
    there and into the ceph_osdc_build_request(). Nothing prevents
    moving these calls to the top of ceph_osdc_build_request(), either
    (and we're going to want them there in the next patch) so put them
    at the top.

    This and the next patch are related to:
    http://tracker.ceph.com/issues/4657

    Signed-off-by: Alex Elder
    Reviewed-by: Josh Durgin

    Alex Elder
     
  • This simply moves ceph_osdc_build_request() later in its source
    file without any change. Done as a separate patch to facilitate
    review of the change in the next patch.

    Signed-off-by: Alex Elder
    Reviewed-by: Josh Durgin

    Alex Elder
     
  • An object class method is formatted using a pagelist which contains
    the class name, the method name, and the data concatenated into an
    osd request's outbound data.

    Currently when a class op is initialized in osd_req_op_cls_init(),
    the lengths of and pointers to these three items are recorded.
    Later, when the op is getting formatted into the request message, a
    new pagelist is created and that is when these items get copied into
    the pagelist.

    This patch makes it so the pagelist to hold these items is created
    when the op is initialized instead.

    Signed-off-by: Alex Elder
    Reviewed-by: Josh Durgin

    Alex Elder
     
  • An osd request now holds all of its source op structures, and every
    place that initializes one of these is in fact initializing one
    of the entries in the the osd request's array.

    So rather than supplying the address of the op to initialize, have
    caller specify the osd request and an indication of which op it
    would like to initialize. This better hides the details the
    op structure (and faciltates moving the data pointers they use).

    Since osd_req_op_init() is a common routine, and it's not used
    outside the osd client code, give it static scope. Also make
    it return the address of the specified op (so all the other
    init routines don't have to repeat that code).

    Signed-off-by: Alex Elder
    Reviewed-by: Josh Durgin

    Alex Elder
     
  • An extent type osd operation currently implies that there will
    be corresponding data supplied in the data portion of the request
    (for write) or response (for read) message. Similarly, an osd class
    method operation implies a data item will be supplied to receive
    the response data from the operation.

    Add a ceph_osd_data pointer to each of those structures, and assign
    it to point to eithre the incoming or the outgoing data structure in
    the osd message. The data is not always available when an op is
    initially set up, so add two new functions to allow setting them
    after the op has been initialized.

    Begin to make use of the data item pointer available in the osd
    operation rather than the request data in or out structure in
    places where it's convenient. Add some assertions to verify
    pointers are always set the way they're expected to be.

    This is a sort of stepping stone toward really moving the data
    into the osd request ops, to allow for some validation before
    making that jump.

    This is the first in a series of patches that resolve:
    http://tracker.ceph.com/issues/4657

    Signed-off-by: Alex Elder
    Reviewed-by: Josh Durgin

    Alex Elder
     
  • There are fields "indata" and "indata_len" defined the ceph osd
    request op structure. The "in" part is with from the point of view
    of the osd server, but is a little confusing here on the client
    side. Change their names to use "request" instead of "in" to
    indicate that it defines data provided with the request (as opposed
    the data returned in the response).

    Rename the local variable in osd_req_encode_op() to match.

    Signed-off-by: Alex Elder
    Reviewed-by: Josh Durgin

    Alex Elder
     
  • An osd request keeps a pointer to the osd operations (ops) array
    that it builds in its request message.

    In order to allow each op in the array to have its own distinct
    data, we will need to keep track of each op's data, and that
    information does not go over the wire.

    As long as we're tracking the data we might as well just track the
    entire (source) op definition for each of the ops. And if we're
    doing that, we'll have no more need to keep a pointer to the
    wire-encoded version.

    This patch makes the array of source ops be kept with the osd
    request structure, and uses that instead of the version encoded in
    the message in places where that was previously used. The array
    will be embedded in the request structure, and the maximum number of
    ops we ever actually use is currently 2. So reduce CEPH_OSD_MAX_OP
    to 2 to reduce the size of the structure.

    The result of doing this sort of ripples back up, and as a result
    various function parameters and local variables become unnecessary.

    Make r_num_ops be unsigned, and move the definition of struct
    ceph_osd_req_op earlier to ensure it's defined where needed.

    It does not yet add per-op data, that's coming soon.

    This resolves:
    http://tracker.ceph.com/issues/4656

    Signed-off-by: Alex Elder
    Reviewed-by: Josh Durgin

    Alex Elder
     
  • One more osd data helper, which returns the length of the
    data item, regardless of its type.

    Signed-off-by: Alex Elder
    Reviewed-by: Josh Durgin

    Alex Elder