From patchwork Fri Aug 24 16:36:10 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Alex Elder X-Patchwork-Id: 1372201 Return-Path: X-Original-To: patchwork-ceph-devel@patchwork.kernel.org Delivered-To: patchwork-process-083081@patchwork2.kernel.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by patchwork2.kernel.org (Postfix) with ESMTP id DBB33DF28C for ; Fri, 24 Aug 2012 16:36:14 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S964923Ab2HXQgN (ORCPT ); Fri, 24 Aug 2012 12:36:13 -0400 Received: from mail-iy0-f174.google.com ([209.85.210.174]:49774 "EHLO mail-iy0-f174.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S964839Ab2HXQgM (ORCPT ); Fri, 24 Aug 2012 12:36:12 -0400 Received: by mail-iy0-f174.google.com with SMTP id o24so3740650ial.19 for ; Fri, 24 Aug 2012 09:36:12 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20120113; h=message-id:date:from:user-agent:mime-version:to:subject:references :in-reply-to:content-type:content-transfer-encoding :x-gm-message-state; bh=eCY0AhgBa2BZCYROFwyIoqrNvm2CG1z3f3HR2UM6v+I=; b=WHywXVaIYed4ifEUVZx7YXETbPO+w7lx0B/YdFcpoUbL79c9LWsr/5bqLJpNACaIWp Ea41oLBEseKqs/8csM8Dw9uqpDdzVR2IXBUf1ZY/kaPdVkJ3nRRLXmqfPusTMmuuJlxR W4tOkNHOlmZvault3HBET/JP9BmmSYNQu5wjza1DziZvNmU/OIvP7yarpc0AY8D5T3eO nmb68BwREaTJgRktgapeOPuJnroFUGrQuTSp34gwiWF17+T6Txz7av9+UWyV50kQLmq9 NRpni3UQXK6uC67svgk9Ck4OWAsanN8xEwocr70UkTrhKBHjk33DhKti7zY46DorFzlx maQQ== Received: by 10.50.152.141 with SMTP id uy13mr2803424igb.64.1345826172435; Fri, 24 Aug 2012 09:36:12 -0700 (PDT) Received: from [172.22.22.4] (c-71-195-31-37.hsd1.mn.comcast.net. [71.195.31.37]) by mx.google.com with ESMTPS id ul4sm209978igb.15.2012.08.24.09.36.11 (version=SSLv3 cipher=OTHER); Fri, 24 Aug 2012 09:36:11 -0700 (PDT) Message-ID: <5037AD7A.4080300@inktank.com> Date: Fri, 24 Aug 2012 11:36:10 -0500 From: Alex Elder User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:14.0) Gecko/20120714 Thunderbird/14.0 MIME-Version: 1.0 To: ceph-devel@vger.kernel.org Subject: [PATCH 10/11] rbd: check for overflow in rbd_get_num_segments() References: <5037AB20.4000103@inktank.com> In-Reply-To: <5037AB20.4000103@inktank.com> X-Gm-Message-State: ALoCoQnvAdGIMXIBhkxoQPpuWyQg2L2pEpdTDXCwSCbMfBrpSPHL4GXAmq0QSlxZREtW0RAj83cm Sender: ceph-devel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: ceph-devel@vger.kernel.org It is possible in rbd_get_num_segments() for an overflow to occur when adding the offset and length. This is easily avoided. Since the function returns an int and the one caller is already prepared to handle errors, have it return -ERANGE if overflow would occur. The overflow check would not work if a zero-length request was being tested, so short-circuit that case, returning 0 for the number of segments required. (This condition might be avoided elsewhere already, I don't know.) Have the caller end the request if either an error or 0 is returned. The returned value is passed to __blk_end_request_all(), meaning a 0 length request is not treated an error. Signed-off-by: Alex Elder Reviewed-by: Yehuda Sadeh --- drivers/block/rbd.c | 23 +++++++++++++++++++++-- 1 file changed, 21 insertions(+), 2 deletions(-) @@ -1502,6 +1515,12 @@ static void rbd_rq_fn(struct request_queue *q) size, (unsigned long long) blk_rq_pos(rq) * SECTOR_SIZE); num_segs = rbd_get_num_segments(&rbd_dev->header, ofs, size); + if (num_segs <= 0) { + spin_lock_irq(q->queue_lock); + __blk_end_request_all(rq, num_segs); + ceph_put_snap_context(snapc); + continue; + } coll = rbd_alloc_coll(num_segs); if (!coll) { spin_lock_irq(q->queue_lock); diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c index fad4ecb..b649446 100644 --- a/drivers/block/rbd.c +++ b/drivers/block/rbd.c @@ -50,6 +50,10 @@ #define SECTOR_SHIFT 9 #define SECTOR_SIZE (1ULL << SECTOR_SHIFT) +/* It might be useful to have this defined elsewhere too */ + +#define U64_MAX ((u64) (~0ULL)) + #define RBD_DRV_NAME "rbd" #define RBD_DRV_NAME_LONG "rbd (rados block device)" @@ -678,8 +682,17 @@ static u64 rbd_get_segment(struct rbd_image_header *header, static int rbd_get_num_segments(struct rbd_image_header *header, u64 ofs, u64 len) { - u64 start_seg = ofs >> header->obj_order; - u64 end_seg = (ofs + len - 1) >> header->obj_order; + u64 start_seg; + u64 end_seg; + + if (!len) + return 0; + if (len - 1 > U64_MAX - ofs) + return -ERANGE; + + start_seg = ofs >> header->obj_order; + end_seg = (ofs + len - 1) >> header->obj_order; + return end_seg - start_seg + 1; }