From patchwork Thu Jun 4 12:43:14 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kinglong Mee X-Patchwork-Id: 6546531 Return-Path: X-Original-To: patchwork-linux-nfs@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork1.web.kernel.org (Postfix) with ESMTP id 47D349F326 for ; Thu, 4 Jun 2015 12:43:27 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 4DEA7207B8 for ; Thu, 4 Jun 2015 12:43:26 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 1A2F720788 for ; Thu, 4 Jun 2015 12:43:25 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752341AbbFDMnY (ORCPT ); Thu, 4 Jun 2015 08:43:24 -0400 Received: from mail-qc0-f182.google.com ([209.85.216.182]:33828 "EHLO mail-qc0-f182.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752335AbbFDMnX (ORCPT ); Thu, 4 Jun 2015 08:43:23 -0400 Received: by qcej9 with SMTP id j9so17000399qce.1 for ; Thu, 04 Jun 2015 05:43:22 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=message-id:date:from:user-agent:mime-version:to:cc:subject :references:in-reply-to:content-type:content-transfer-encoding; bh=SNEtL0QPHMPWDikRJB+70j6zW2Jl0GweUI/5uwwFW4c=; b=mRpLW/wDUCYepLGUwO+LIwI/jbyWW/EhfXDdGo6DL2yLjav9tLHiOU/tpj5vfFGENW Vo6+fy8MBIrNy8iXAyKCEl4piL13qOpNbjqESnkdjbe8rKnN3XTIvKYPogcrAe3LcZd8 9th0lt3lnT4MAyx4OhGmM+f5nTEFdSpt3wbAJ9UF/m95cQ4UdYYVZudYIrVaGwmgtl8c NAESV0AjilP3BdvmMb/AOGMGFg1DzrppHFC0WJavlUArLtteVtbsiuPVQSveEgwzEapJ hVaCm6hC6HKhgOLCwn7K/wqzy7dcSryZioNLWa5BrzyTzkBhZ9CVySARXupU6mawqdmv v6YQ== X-Received: by 10.55.19.12 with SMTP id d12mr71315907qkh.89.1433421802671; Thu, 04 Jun 2015 05:43:22 -0700 (PDT) Received: from [192.168.99.13] ([104.143.41.79]) by mx.google.com with ESMTPSA id r31sm2298571qkr.42.2015.06.04.05.43.18 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 04 Jun 2015 05:43:21 -0700 (PDT) Message-ID: <557047E2.10804@gmail.com> Date: Thu, 04 Jun 2015 20:43:14 +0800 From: Kinglong Mee User-Agent: Mozilla/5.0 (Windows NT 6.3; WOW64; rv:31.0) Gecko/20100101 Thunderbird/31.7.0 MIME-Version: 1.0 To: Stanislav Kholmanskikh , "J. Bruce Fields" CC: linux-nfs@vger.kernel.org, Vasily Isaenko , "SHUANG.QIU" Subject: Re: nfsd: EACCES vs EPERM on utime()/utimes() calls References: <556C73AE.4090900@oracle.com> <20150601212317.GF26972@fieldses.org> <556DD52D.5040405@oracle.com> In-Reply-To: <556DD52D.5040405@oracle.com> Sender: linux-nfs-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-nfs@vger.kernel.org X-Spam-Status: No, score=-6.8 required=5.0 tests=BAYES_00, DKIM_ADSP_CUSTOM_MED, DKIM_SIGNED, FREEMAIL_FROM, RCVD_IN_DNSWL_HI, T_DKIM_INVALID, T_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 6/3/2015 12:09 AM, Stanislav Kholmanskikh wrote: > On 06/02/2015 12:23 AM, bfields@fieldses.org wrote: >> On Mon, Jun 01, 2015 at 06:01:02PM +0300, Stanislav Kholmanskikh wrote: >>> Hello. >>> >>> As the man page for utime/utimes states [1], EPERM is returned if >>> the second argument of utime/utimes is not NULL and: >>> * the caller's effective user id does not match the owner of the file >>> * the caller does not have write access to the file >>> * the caller is not privileged >>> >>> However, I don't see this behavior with NFS, I see EACCES is >>> generated instead. >> >> Agreed that it's probably a server bug. (Have you run across a case >> where this makes a difference?) > > Thank you. > > No, I've not seen such a real-word scenario. > > I have these LTP test cases failing: > > * https://github.com/linux-test-project/ltp/blob/master/testcases/kernel/syscalls/utime/utime06.c > * https://github.com/linux-test-project/ltp/blob/master/testcases/kernel/syscalls/utimes/utimes01.c > > and it makes me a bit nervous :) > >> >> Looking at nfsd_setattr().... The main work is done by notify_change(), >> which is probably doing the right thing. But before that there's an >> fh_verify()--looks like that is expected to fail in your case. I bet >> that's the cause. Yes, it is. nfsd do the permission checking before notify_change() as, /* This assumes NFSD_MAY_{READ,WRITE,EXEC} == MAY_{READ,WRITE,EXEC} */ err = inode_permission(inode, acc & (MAY_READ|MAY_WRITE|MAY_EXEC)); return -EACCES for non-owner user. > > I doubt I can fix it by myself (at least quickly). So I am happy if anyone more experienced will look at it as well :) > > Anyway, if nobody is interested, I'll give it a try, but later. Here is a diff patch for this problem, please try testing. If okay, I will send an official patch. Note: must apply the following patch first in the url, http://git.linux-nfs.org/?p=bfields/linux.git;a=commitdiff;h=cc265089ce1b176dde963c74b53593446ee7f99a thanks, Kinglong Mee ------------------------------------------------------------- --- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c index 84d770b..2533088 100644 --- a/fs/nfsd/vfs.c +++ b/fs/nfsd/vfs.c @@ -407,10 +371,23 @@ nfsd_setattr(struct svc_rqst *rqstp, struct svc_fh *fhp, struct iattr *iap, bool get_write_count; int size_change = 0; - if (iap->ia_valid & (ATTR_ATIME | ATTR_MTIME | ATTR_SIZE)) + if (iap->ia_valid & ATTR_SIZE) { accmode |= NFSD_MAY_WRITE|NFSD_MAY_OWNER_OVERRIDE; - if (iap->ia_valid & ATTR_SIZE) ftype = S_IFREG; + } + + /* + * According to utimes_common(), + * + * If times is NULL (or both times are UTIME_NOW), + * then we need to check permissions, because + * inode_change_ok() won't do it. + */ + if (iap->ia_valid & (ATTR_ATIME | ATTR_MTIME)) { + accmode |= NFSD_MAY_OWNER_OVERRIDE; + if (!(iap->ia_valid & (ATTR_ATIME_SET | ATTR_MTIME_SET))) + accmode |= NFSD_MAY_WRITE; + } /* Callers that do fh_verify should do the fh_want_write: */ get_write_count = !fhp->fh_dentry;