[bug report] fs/ntfs3: inconsistent NULL checking in attr_allocate_frame()

[bug report] fs/ntfs3: inconsistent NULL checking in attr_allocate_frame()

[Dan Carpenter]

Hello Konstantin Komarov,

This is a semi-automatic email about new static checker warnings.

The patch be71b5cba2e6: "fs/ntfs3: Add attrib operations" from Aug 
13, 2021, leads to the following Smatch complaint:

    fs/ntfs3/attrib.c:1678 attr_allocate_frame()
    error: we previously assumed 'attr_b' could be null (see line 1591)

fs/ntfs3/attrib.c
  1582                  /* add new segment [next_svcn : evcn1 - next_svcn )*/
  1583                  if (!ni->attr_list.size) {
  1584                          err = ni_create_attr_list(ni);
  1585                          if (err)
  1586                                  goto out;
  1587                          /* layout of records is changed */
  1588                          le_b = NULL;
  1589                          attr_b = ni_find_attr(ni, NULL, &le_b, ATTR_DATA, NULL,
  1590						      0, NULL, &mi_b);
  1591				if (!attr_b) {
                                     ^^^^^^
Check for NULL

  1592					err = -ENOENT;
  1593					goto out;
  1594				}
  1595	
  1596				attr = attr_b;
  1597				le = le_b;
  1598				mi = mi_b;
  1599				goto repack;
  1600			}
  1601		}
  1602	
  1603		svcn = evcn1;
  1604	
  1605		/* Estimate next attribute */
  1606		attr = ni_find_attr(ni, attr, &le, ATTR_DATA, NULL, 0, &svcn, &mi);
  1607	
  1608		if (attr) {
  1609			CLST alloc = bytes_to_cluster(
  1610				sbi, le64_to_cpu(attr_b->nres.alloc_size));
  1611			CLST evcn = le64_to_cpu(attr->nres.evcn);
  1612	
  1613			if (end < next_svcn)
  1614				end = next_svcn;
  1615			while (end > evcn) {
  1616				/* remove segment [svcn : evcn)*/
  1617				mi_remove_attr(mi, attr);
  1618	
  1619				if (!al_remove_le(ni, le)) {
  1620					err = -EINVAL;
  1621					goto out;
  1622				}
  1623	
  1624				if (evcn + 1 >= alloc) {
  1625					/* last attribute segment */
  1626					evcn1 = evcn + 1;
  1627					goto ins_ext;
  1628				}
  1629	
  1630				if (ni_load_mi(ni, le, &mi)) {
  1631					attr = NULL;
  1632					goto out;
  1633				}
  1634	
  1635				attr = mi_find_attr(mi, NULL, ATTR_DATA, NULL, 0,
  1636						    &le->id);
  1637				if (!attr) {
  1638					err = -EINVAL;
  1639					goto out;
  1640				}
  1641				svcn = le64_to_cpu(attr->nres.svcn);
  1642				evcn = le64_to_cpu(attr->nres.evcn);
  1643			}
  1644	
  1645			if (end < svcn)
  1646				end = svcn;
  1647	
  1648			err = attr_load_runs(attr, ni, run, &end);
  1649			if (err)
  1650				goto out;
  1651	
  1652			evcn1 = evcn + 1;
  1653			attr->nres.svcn = cpu_to_le64(next_svcn);
  1654			err = mi_pack_runs(mi, attr, run, evcn1 - next_svcn);
  1655			if (err)
  1656				goto out;
  1657	
  1658			le->vcn = cpu_to_le64(next_svcn);
  1659			ni->attr_list.dirty = true;
  1660			mi->dirty = true;
  1661	
  1662			next_svcn = le64_to_cpu(attr->nres.evcn) + 1;
  1663		}
  1664	ins_ext:
  1665		if (evcn1 > next_svcn) {
  1666			err = ni_insert_nonresident(ni, ATTR_DATA, NULL, 0, run,
  1667						    next_svcn, evcn1 - next_svcn,
  1668						    attr_b->flags, &attr, &mi);
  1669			if (err)
  1670				goto out;
  1671		}
  1672	ok:
  1673		run_truncate_around(run, vcn);
  1674	out:
  1675		if (new_valid > data_size)
  1676			new_valid = data_size;
  1677	
  1678		valid_size = le64_to_cpu(attr_b->nres.valid_size);
                                         ^^^^^^^^^^^^^^^^^^^^^^^
Unchecked dereference

  1679		if (new_valid != valid_size) {
  1680			attr_b->nres.valid_size = cpu_to_le64(valid_size);

regards,
dan carpenter