[lvc-project] [PATCH] ocfs2: always run deallocs on copy-on-write completion

[Joseph Qi]

On 7/21/26 5:08 PM, Dmitry Antipov wrote:
> On 7/21/26 4:35 AM, Joseph Qi wrote:
> 
>> The identical idiom also exists in other places:
>>    ocfs2_attach_refcount_tree
>>    ocfs2_create_reflink_node
>>    ocfs2_reflink_remap_blocks
>>    ocfs2_reflink_xattrs
>>
>> So please check if it will leak the same way. If so, please apply the
>> same changes as well.
> 
> So the basic problem is to check whether 'ocfs2_cache_block_dealloc()'
> can be called from these functions as well. Since I have the only
> .syz reproducer that proves that 'ocfs2_cache_block_dealloc()' can
> be called from 'ocfs2_replace_cow()', time to try some static analysis.
> 
> 1. Collect LLVM bitcode of the relevant source code:
> 
> diff --git a/fs/ocfs2/Makefile b/fs/ocfs2/Makefile
> index cc9b32b9db7c..6163df78e873 100644
> --- a/fs/ocfs2/Makefile
> +++ b/fs/ocfs2/Makefile
> @@ -1,5 +1,5 @@
>  # SPDX-License-Identifier: GPL-2.0
> -ccflags-y := -I$(src)
> +ccflags-y := -I$(src) --save-temps=obj
> 
>  obj-$(CONFIG_OCFS2_FS) +=      \
>         ocfs2.o                 \
> 
> 2. Link them into a single unified bitcode and produce call graph:
> 
> $ llvm-link $(find fs/ocfs2 -name '*.bc') -o ocfs2.bc
> $ opt --passes=dot-callgraph ocfs2.bc
> 
> This produces ocfs2.bc.callgraph.dot.
> 
> 3. Use the following gvpr(1) (comes from graphviz) script:
> 
> // AI-assisted breadth-first search
> 
> BEGIN {
>     if (ARGC < 2) {
>         printf(2, "Error: Please provide source and target node names as arguments.\n");
>         printf(2, "Usage: gvpr -f reachable.gvpr -a [source] -a [target] input.gv\n");
>         exit(1);
>     }
>     string src = ARGV[0];
>     string tgt = ARGV[1];
> 
>     // Arrays for tracking BFS
>     node_t queue[];
>     int visited[string];
>     int Head = 0;
>     int Tail = 0;
> }
> 
> BEG_G {
>     node_t s = node($, src);
>     node_t t = node($, tgt);
> 
>     if (s == NULL || t == NULL) {
>         printf(2, "Error: One or both nodes do not exist in the graph.\n");
>         exit(1);
>     }
> 
>     if (s == t) {
>         print("yes (source is equal to target)");
>         exit(0);
>     }
> 
>     // Initialize BFS queue
>     queue[Tail++] = s;
>     visited[s.name] = 1;
> 
>     // Run BFS loop
>     while (Head < Tail) {
>         node_t curr = queue[Head++];
>         edge_t e;
> 
>         // Loop through all outgoing edges of the current node
>         for (e = fstedge(curr); e; e = nxtedge(e, curr)) {
>             if (e.head == curr) continue;
> 
>             node_t nextNode = e.head;
>             if (!visited[nextNode.name]) {
>                 if (nextNode == t) {
>                     print("yes");
>                     exit(0);
>                 }
>                 visited[nextNode.name] = 1;
>                 queue[Tail++] = nextNode;
>             }
>         }
>     }
> 
>     print("no");
> }
> 
> and simple shell script to check suspected functions:
> 
> #!/bin/bash
> 
> TARGET=ocfs2_cache_block_dealloc
> NODE_target=$(grep "{$TARGET}" ocfs2.bc.callgraph.dot  | awk '{print $1}')
> 
> for NAME in ocfs2_replace_cow \
>         ocfs2_attach_refcount_tree \
>         ocfs2_create_reflink_node \
>         ocfs2_reflink_remap_blocks \
>         ocfs2_reflink_xattrs
> do
>     NODE_check=$(grep "{$NAME}" ocfs2.bc.callgraph.dot  | awk '{print $1}')
>     echo -ne "Check whether $TARGET() is reachable from $NAME()..."
>     gvpr -f reachable.gvpr -a $NODE_check -a $NODE_target ocfs2.bc.callgraph.dot
> done
> 
> This produces the following output:
> 
> Check whether ocfs2_cache_block_dealloc() is reachable from ocfs2_replace_cow()...yes
> Check whether ocfs2_cache_block_dealloc() is reachable from ocfs2_attach_refcount_tree()...yes
> Check whether ocfs2_cache_block_dealloc() is reachable from ocfs2_create_reflink_node()...no
> Check whether ocfs2_cache_block_dealloc() is reachable from ocfs2_reflink_remap_blocks()...yes
> Check whether ocfs2_cache_block_dealloc() is reachable from ocfs2_reflink_xattrs()...no
> 
> So I would assume that 'ocfs2_create_reflink_node()' and 'ocfs2_reflink_xattrs()'
> are better to leave untouched.
> 
Reasons for ocfs2_create_reflink_node() and ocfs2_reflink_xattrs() are
marked unreachable:
ocfs2_create_reflink_node() - it's purely additive without remove blocks.
ocfs2_reflink_xattrs() - allocate new xattr blocks for the destination, with
the source's xattr tree untouched.

Since ocfs2_run_deallocs() is a no-op with an empty dealloc context, so it
seems marking it unconditional is safe. And this will keep the code pattern
consistent.

Thanks,
Joseph