@@ -670,12 +670,24 @@ xfs_attri_item_relog(
new_attrp->alfi_name_len = old_attrp->alfi_name_len;
new_attrp->alfi_attr_filter = old_attrp->alfi_attr_filter;
- memcpy(new_attrip->attri_name, old_attrip->attri_name,
- new_attrip->attri_name_len);
+ if (old_attrip->attri_name) {
+ memcpy(new_attrip->attri_name, old_attrip->attri_name,
+ new_attrip->attri_name_len);
+ } else {
+ xfs_emerg(tp->t_mountp, "%s namelen 0x%x name NULL!", __func__, new_attrip->attri_name_len);
+ dump_stack();
+ }
- if (new_attrip->attri_value_len > 0)
- memcpy(new_attrip->attri_value, old_attrip->attri_value,
- new_attrip->attri_value_len);
+ if (new_attrip->attri_value_len > 0) {
+ if (old_attrip->attri_value) {
+ memcpy(new_attrip->attri_value,
+ old_attrip->attri_value,
+ new_attrip->attri_value_len);
+ } else {
+ xfs_emerg(tp->t_mountp, "%s valuelen 0x%x value NULL!", __func__, new_attrip->attri_value_len);
+ dump_stack();
+ }
+ }
xfs_trans_add_item(tp, &new_attrip->attri_item);
set_bit(XFS_LI_DIRTY, &new_attrip->attri_item.li_flags);
From: Darrick J. Wong <djwong@kernel.org> XXX DO NOT APPLY While running xfs/297 and generic/642, I noticed a crash in xfs_attri_item_relog when it tries to copy the attr name to the new xattri log item. I think what happened here was that we called ->iop_commit on the old attri item (which nulls out the pointers) as part of a log force at the same time that a chained attr operation was ongoing. The system was busy enough that at some later point, the defer ops operation decided it was necessary to relog the attri log item, but as we've detached the name buffer from the old attri log item, we can't copy it to the new one, and kaboom. I think speaks to a broader refcounting problem with LARP mode -- the attr log item needs to be able to retain a reference to the name and value buffers until the log items have completely cleared the log. I think it might be possible that the setxattr code can return to userspace before the CIL actually formats and commits the log item, leading to a UAF, but I don't really have the time to figure that one out without external help. Skipping the memcpy is /not/ the correct solution here -- that means we'll relog the xattri with zeroed names and values, which breaks log recovery. Singed-off-by: Darrick J. Wong <djwong@kernel.org> --- fs/xfs/xfs_attr_item.c | 22 +++++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-)