From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751905AbdFFLei (ORCPT ); Tue, 6 Jun 2017 07:34:38 -0400 Received: from mx0a-001b2d01.pphosted.com ([148.163.156.1]:33461 "EHLO mx0a-001b2d01.pphosted.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751702AbdFFLeg (ORCPT ); Tue, 6 Jun 2017 07:34:36 -0400 Subject: Re: [Linux-ima-devel] [PATCH 5/7] ima: add securityfs interface to save a measurements list with kexec header From: Mimi Zohar To: Roberto Sassu , linux-ima-devel@lists.sourceforge.net Cc: linux-security-module@vger.kernel.org, linux-kernel@vger.kernel.org Date: Tue, 06 Jun 2017 07:33:39 -0400 In-Reply-To: <25671f22-7145-6ff3-40c0-a27665443a2c@huawei.com> References: <20170516125347.10574-1-roberto.sassu@huawei.com> <20170516125347.10574-6-roberto.sassu@huawei.com> <1496642698.2998.104.camel@linux.vnet.ibm.com> <25671f22-7145-6ff3-40c0-a27665443a2c@huawei.com> Content-Type: text/plain; charset="UTF-8" X-Mailer: Evolution 3.20.5 (3.20.5-1.fc24) Mime-Version: 1.0 Content-Transfer-Encoding: 8bit X-TM-AS-MML: disable x-cbid: 17060611-0004-0000-0000-0000020E82B9 X-IBM-AV-DETECTION: SAVI=unused REMOTE=unused XFE=unused x-cbparentid: 17060611-0005-0000-0000-00000A05B99C Message-Id: <1496748819.5546.42.camel@linux.vnet.ibm.com> X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:,, definitions=2017-06-06_09:,, signatures=0 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 spamscore=0 suspectscore=0 malwarescore=0 phishscore=0 adultscore=0 bulkscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1703280000 definitions=main-1706060191 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, 2017-06-06 at 11:13 +0200, Roberto Sassu wrote: > >>  /* returns pointer to hlist_node */ > >>  static void *ima_measurements_start(struct seq_file *m, loff_t *pos) > >>  { > >>      loff_t l = *pos; > >>      struct ima_queue_entry *qe; > >> +    struct ima_queue_entry *qe_found = NULL; > >> +    unsigned long size = 0, count = 0; > >> +    bool khdr = m->file->f_path.dentry == binary_kexec_runtime_measurements; > >> > >>      /* we need a lock since pos could point beyond last element */ > >>      rcu_read_lock(); > >>      list_for_each_entry_rcu(qe, &ima_measurements, later) { > >> -            if (!l--) { > >> -                    rcu_read_unlock(); > >> -                    return qe; > >> +            if (!l) { > >> +                    qe_found = qe_found ? qe_found : qe; > > > > What is this? > > ima_measurements_start() should return the list entry at position *pos. > The line above prevents qe_found from being updated when the loop > continues until the last list entry. Wouldn't a simple if/then be more appropriate here? > > > > >> + > >> +                    if (!khdr) > >> +                            break; > > > > Does this test need to be in the loop? > > Yes. Otherwise, ima_measurements_start() would iterate over the whole > list when it is not necessary. Oh, for displaying the measurement list you need to set qe_found before returning. thanks, Mimi