Received-SPF: pass (google.com: domain of linux-nfs-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Tue, 7 Sep 2021 11:07:32 +0300
From:   Dan Carpenter <dan.carpenter@oracle.com>
To:     jlayton@kernel.org
Cc:     linux-nfs@vger.kernel.org
Subject: [bug report] nfsd: Protect session creation and client confirm using
 client_lock
Message-ID: <20210907080732.GA20341@kili>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
User-Agent: Mutt/1.10.1 (2018-07-13)
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?A1HEuMJPZmbAxF3ntmVO8jUrnquUBmvoSBBMwGpDd68yAkh8CX1Xwgl+YeeO?=
 =?us-ascii?Q?rBft7wj+AaR1zLlTBN+nkixUdv2aykupEwGiBkaofT/5pD8YA6u3aCkFdOBf?=
 =?us-ascii?Q?498z5YCo073vnz/IjFE+V+41m057GhX6ZG1FdcJ9A49uCj6V2BuvSHpQRXrz?=
 =?us-ascii?Q?JOAIYZT7CnLvFrqg+PPBScc6HcpHRZF5YRjyhx5Bw+JuehLxYCLQbFGNh9K7?=
 =?us-ascii?Q?Qj77m62INs+N9jdaTMo9P0cJSOkP6iyJ7vkyL7FnLankBYShMO7EMdsf5MTY?=
 =?us-ascii?Q?VC6lZG5AjSH2tPPwz88hJZB3omFBLquFJCvTdpiClrVc7LZio5dbCa5l8IjF?=
 =?us-ascii?Q?nSSU6eWM66QCRoo2Hg91x0WPoA1p/cZZGb7nJ4LDWuEJ0+3PSlqt5hKycgp7?=
 =?us-ascii?Q?NvdM5YlAqFTuGa1W3tp5NEmCChFOcT4Jqx53toD080Cx8Yig32qQAnjvbUNo?=
 =?us-ascii?Q?tzkc6ZcJ9I9MEmLiWDrviLYSP/3RhcsiwZPGlQcpM6tq1ih08MVuUQY7MwGQ?=
 =?us-ascii?Q?81tbh0H9cL975qtsiMhU6XEmg1Yof/nJHk/C5IkF1YUSrBtzjpBzJlVmwqIZ?=
 =?us-ascii?Q?cFBKdWjXviQJMw1pFebh3QSia7XGvdYTC2eB3qU+kLfysRZxUiatqTbpmF5O?=
 =?us-ascii?Q?8/gNjCeRbQyQ2mhwWB2kLmQPLQxVZvVkgieOXzYRTjil3vxXi6hG8402xWm9?=
 =?us-ascii?Q?JIF09gKABfwkSzptT9sVe9CsjqQjlM8ld71/BU6ybXL561yyLLrR0Xgk04fc?=
 =?us-ascii?Q?OcTt3Lk3KjNHjrspD8lVHjxrLxdyOwMWihMxOlxv0REQU6BrTtoOJJkbdHUG?=
 =?us-ascii?Q?Gu0j05X7L01H+DRDfUOZ5dC8SzfyhmoNKYUjFC3X98M/A1GVvn6H/954HQSn?=
 =?us-ascii?Q?BRoc6VUX6lcFFaWPvJ73g+bj0yy2Cxfe7CbN93Nzm5RJ+tTePyfulVx2YJ7x?=
 =?us-ascii?Q?NQ3oamLYseUsXZEZ5c7GM30Cm0yWGvkEH2lfjJtWJy7xiuZWvXHgbcalzFxe?=
 =?us-ascii?Q?i2kTFwN6PZ6ztApoFr73QW2v4UWxpK0RnCXygpyarG+FXdxUUkQYXYVj2jEL?=
 =?us-ascii?Q?8Yg3LOTKLO9IBd4P9hVFyP8zKtX3iAeV8Da1LNOl9180UDF+A0bYzEeP/a3/?=
 =?us-ascii?Q?QwcSPvtCaDP5Zu+XRDy4ZGBBWKg+rU+bSg8U1JefNSVKTpbhjrfb6z7/altd?=
 =?us-ascii?Q?ysB1KFSbaNriSkv83AOSTpoYM/BT29wgm5ty8M43aazwNwduKpLeG5cnT8Xw?=
 =?us-ascii?Q?31QGzQYtT7sq+q1hkSS+bb4BFNDUvwPfPjWSRPX5XALEZFIeI2wZCzfT/vYl?=
 =?us-ascii?Q?EJDl/KkdWfAEwefLOphobUOk?=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 3708e896-9f3e-4cbc-4769-08d971d68f80
X-MS-Exchange-CrossTenant-AuthSource: MWHPR1001MB2365.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 07 Sep 2021 08:07:39.5713
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 4e2c6054-71cb-48f1-bd6c-3a9705aca71b
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: nTyf6vTB8IUXW8N0J82cQHk56upTGQFfu7rEzCrwkFc/emCqLcxsb7cEM704E8rECaLqZ5X0xfQZmxKp1v7EfTkrJdSIktkp11I9d/3+q4s=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MWHPR10MB1904
X-Proofpoint-Virus-Version: vendor=nai engine=6300 definitions=10099 signatures=668682
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 mlxscore=0 spamscore=0 mlxlogscore=999
 adultscore=0 bulkscore=0 suspectscore=0 malwarescore=0 phishscore=0
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2108310000
 definitions=main-2109070053
X-Proofpoint-GUID: PQkz1MTDMMueLCATnBKSZKR03j3EbAAl
X-Proofpoint-ORIG-GUID: PQkz1MTDMMueLCATnBKSZKR03j3EbAAl
Precedence: bulk
List-ID: <linux-nfs.vger.kernel.org>
X-Mailing-List: linux-nfs@vger.kernel.org

Hello Jeff Layton,

The patch d20c11d86d8f: "nfsd: Protect session creation and client
confirm using client_lock" from Jul 30, 2014, leads to the following
Smatch static checker warning:

	net/sunrpc/addr.c:178 rpc_parse_scope_id()
	warn: sleeping in atomic context

net/sunrpc/addr.c
    161 static int rpc_parse_scope_id(struct net *net, const char *buf,
    162                               const size_t buflen, const char *delim,
    163                               struct sockaddr_in6 *sin6)
    164 {
    165         char *p;
    166         size_t len;
    167 
    168         if ((buf + buflen) == delim)
    169                 return 1;
    170 
    171         if (*delim != IPV6_SCOPE_DELIMITER)
    172                 return 0;
    173 
    174         if (!(ipv6_addr_type(&sin6->sin6_addr) & IPV6_ADDR_LINKLOCAL))
    175                 return 0;
    176 
    177         len = (buf + buflen) - delim - 1;
--> 178         p = kmemdup_nul(delim + 1, len, GFP_KERNEL);
    179         if (p) {
    180                 u32 scope_id = 0;
    181                 struct net_device *dev;
    182 
    183                 dev = dev_get_by_name(net, p);
    184                 if (dev != NULL) {
    185                         scope_id = dev->ifindex;
    186                         dev_put(dev);
    187                 } else {
    188                         if (kstrtou32(p, 10, &scope_id) != 0) {
    189                                 kfree(p);
    190                                 return 0;
    191                         }
    192                 }
    193 
    194                 kfree(p);
    195 
    196                 sin6->sin6_scope_id = scope_id;
    197                 return 1;
    198         }
    199 
    200         return 0;
    201 }

The call tree is:

nfsd4_setclientid() <- disables preempt
-> gen_callback()
   -> rpc_uaddr2sockaddr()
      -> rpc_pton()
         -> rpc_pton6()
            -> rpc_parse_scope_id()

The commit added a spin_lock to nfsd4_setclientid().

fs/nfsd/nfs4state.c
  4023                          trace_nfsd_clid_verf_mismatch(conf, rqstp,
  4024                                                        &clverifier);
  4025          } else
  4026                  trace_nfsd_clid_fresh(new);
  4027          new->cl_minorversion = 0;
  4028          gen_callback(new, setclid, rqstp);
                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

  4029          add_to_unconfirmed(new);
  4030          setclid->se_clientid.cl_boot = new->cl_clientid.cl_boot;
  4031          setclid->se_clientid.cl_id = new->cl_clientid.cl_id;
  4032          memcpy(setclid->se_confirm.data, new->cl_confirm.data, sizeof(setclid->se_confirm.data));
  4033          new = NULL;
  4034          status = nfs_ok;
  4035  out:
  4036          spin_unlock(&nn->client_lock);
                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
This is the new lock.

  4037          if (new)
  4038                  free_client(new);
  4039          if (unconf) {
  4040                  trace_nfsd_clid_expire_unconf(&unconf->cl_clientid);
  4041                  expire_client(unconf);
  4042          }
  4043          return status;

regards,
dan carpenter