From patchwork Thu Sep 11 16:17:56 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ilya Dryomov X-Patchwork-Id: 4889661 Return-Path: X-Original-To: patchwork-ceph-devel@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork2.web.kernel.org (Postfix) with ESMTP id EDD28C0338 for ; Thu, 11 Sep 2014 16:18:11 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 469C52021B for ; Thu, 11 Sep 2014 16:18:07 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id B642920200 for ; Thu, 11 Sep 2014 16:18:05 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757118AbaIKQSD (ORCPT ); Thu, 11 Sep 2014 12:18:03 -0400 Received: from mail-qc0-f169.google.com ([209.85.216.169]:60014 "EHLO mail-qc0-f169.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755808AbaIKQSC (ORCPT ); Thu, 11 Sep 2014 12:18:02 -0400 Received: by mail-qc0-f169.google.com with SMTP id r5so2393858qcx.0 for ; Thu, 11 Sep 2014 09:17:57 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:date :message-id:subject:from:to:cc:content-type; bh=oA7QlQ9hFibpPvqhL5Bd2AFKPTGoYp0oUW5AJgG4R9A=; b=D9NEz/ImqyFkHsMtskPwl/prs9Ipxc6xBC9ALxJ1QOoZTxQlOCdAL7KJKcuzds6UYg iiXzOAmKEB+ClauMWcORl3Nm3mM0cz/J6sCgsE+gHHHRGQwTzAExnYM1AaAg8f81yNCk TF8Jf3nNzie9vUppRDk230rovlup80TS5liuqlLDDJmS4Ils8Wr/WtxaYL11K9km+usi hDYRwxtGsJ1ZzaIf4YHavG8rXJg1azxI61L8J4aDVnMhuxUqMNkFsTx9NV6gEF84MPXM 2Z4TUGLeVs8Iabwc+m4YwyXJwb/n+z0Hm6lTzhX15v7AeLnaX/IPuDhTm7wVd7foPt/e SmVA== X-Gm-Message-State: ALoCoQlWa4sgo6/wG2CVLl0zdDRuL4Z1nBO0uD4bH2Nj2Y4fUgpizqkAH0tDQ4CS/Wq8VrFOiSDe MIME-Version: 1.0 X-Received: by 10.224.173.7 with SMTP id n7mr2998035qaz.15.1410452277168; Thu, 11 Sep 2014 09:17:57 -0700 (PDT) Received: by 10.140.94.114 with HTTP; Thu, 11 Sep 2014 09:17:56 -0700 (PDT) In-Reply-To: <5411BE6F.1040202@ieee.org> References: <1410448246-6635-1-git-send-email-ilya.dryomov@inktank.com> <5411BE6F.1040202@ieee.org> Date: Thu, 11 Sep 2014 20:17:56 +0400 Message-ID: Subject: Re: [PATCH] rbd: do not return -ERANGE on auth failure From: Ilya Dryomov To: Alex Elder Cc: Ceph Development Sender: ceph-devel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: ceph-devel@vger.kernel.org X-Spam-Status: No, score=-9.4 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_HI, RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=ham version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On Thu, Sep 11, 2014 at 7:23 PM, Alex Elder wrote: > On 09/11/2014 10:10 AM, Ilya Dryomov wrote: >> Trying to map an image out of a pool for which we don't have an 'x' >> permission bit fails with -ERANGE from ceph_extract_encoded_string() >> due to an unsigned vs signed bug. Fix it and get rid of the -ENIVAL >> sink, thus exposing rbd::get_id cls method return value. (I've seen >> a bunch of unexplained -ERANGE reports, I bet this is it). >> >> Signed-off-by: Ilya Dryomov > > I often think people are annoyed by my explicit type casts > all over the place. This (missed) one matters a lot. > > I think the -EINVAL was to ensure an error code that was > expected by a write() call would be returned. Yeah, the way it's written it's possible in theory to get a positive return value from rbd_dev_image_id(). Looking deeper, this sizeof() is not needed at all - ceph_extract_encoded_string() deals with short buffers as it should. As for the ret == sizeof(u32) (i.e. an empty string), neither userspace nor us check against empty strings in similar cases (object prefix, snapshot name, etc). With the above in mind, how about this? From 3ded0a7fee82f2204c58b4fc00fc74f05331514d Mon Sep 17 00:00:00 2001 From: Ilya Dryomov Date: Thu, 11 Sep 2014 18:49:18 +0400 Subject: [PATCH] rbd: do not return -ERANGE on auth failures Trying to map an image out of a pool for which we don't have an 'x' permission bit fails with -ERANGE from ceph_extract_encoded_string() due to an unsigned vs signed bug. Fix it and get rid of the -EINVAL sink, thus propagating rbd::get_id cls method errors. (I've seen a bunch of unexplained -ERANGE reports, I bet this is it). Signed-off-by: Ilya Dryomov Reviewed-by: Alex Elder --- drivers/block/rbd.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c index 4b97baf8afa3..ce457db5d847 100644 --- a/drivers/block/rbd.c +++ b/drivers/block/rbd.c @@ -4924,7 +4924,7 @@ static int rbd_dev_image_id(struct rbd_device *rbd_dev) ret = image_id ? 0 : -ENOMEM; if (!ret) rbd_dev->image_format = 1; - } else if (ret > sizeof (__le32)) { + } else if (ret >= 0) { void *p = response; image_id = ceph_extract_encoded_string(&p, p + ret, @@ -4932,8 +4932,6 @@ static int rbd_dev_image_id(struct rbd_device *rbd_dev) ret = PTR_ERR_OR_ZERO(image_id); if (!ret) rbd_dev->image_format = 2; - } else { - ret = -EINVAL; } if (!ret) {