Hello Sergey Senozhatsky,
Commit 7ec2cb65ef0d ("zram: unlock slot during recompression") from
Jan 27, 2025 (linux-next), leads to the following Smatch static
checker warning:
drivers/block/zram/zram_drv.c:1960 recompress_slot()
warn: passing freed memory 'zstrm' (line 1943)
drivers/block/zram/zram_drv.c
1873 static int recompress_slot(struct zram *zram, u32 index, struct page *page,
1874 u64 *num_recomp_pages, u32 threshold, u32 prio,
1875 u32 prio_max)
1876 {
1877 struct zcomp_strm *zstrm = NULL;
1878 unsigned long handle_old;
1879 unsigned long handle_new;
1880 unsigned int comp_len_old;
1881 unsigned int comp_len_new;
1882 unsigned int class_index_old;
1883 unsigned int class_index_new;
1884 u32 num_recomps = 0;
1885 void *src, *dst;
1886 int ret;
1887
1888 handle_old = zram_get_handle(zram, index);
1889 if (!handle_old)
1890 return -EINVAL;
1891
1892 comp_len_old = zram_get_obj_size(zram, index);
1893 /*
1894 * Do not recompress objects that are already "small enough".
1895 */
1896 if (comp_len_old < threshold)
1897 return 0;
1898
1899 ret = zram_read_from_zspool(zram, page, index);
1900 if (ret)
1901 return ret;
1902
1903 /*
1904 * We touched this entry so mark it as non-IDLE. This makes sure that
1905 * we don't preserve IDLE flag and don't incorrectly pick this entry
1906 * for different post-processing type (e.g. writeback).
1907 */
1908 zram_clear_flag(zram, index, ZRAM_IDLE);
1909
1910 class_index_old = zs_lookup_class_index(zram->mem_pool, comp_len_old);
1911
1912 /*
1913 * Set prio to one past current slot's compression prio, so that
1914 * we automatically skip lower priority algorithms.
1915 */
1916 prio = zram_get_priority(zram, index) + 1;
1917 /* Slot data copied out - unlock its bucket */
1918 zram_slot_write_unlock(zram, index);
1919 /*
1920 * Iterate the secondary comp algorithms list (in order of priority)
1921 * and try to recompress the page.
1922 */
1923 for (; prio < prio_max; prio++) {
1924 if (!zram->comps[prio])
1925 continue;
1926
1927 num_recomps++;
1928 zstrm = zcomp_stream_get(zram->comps[prio]);
1929 src = kmap_local_page(page);
1930 ret = zcomp_compress(zram->comps[prio], zstrm,
1931 src, &comp_len_new);
1932 kunmap_local(src);
1933
1934 if (ret)
1935 break;
1936
1937 class_index_new = zs_lookup_class_index(zram->mem_pool,
1938 comp_len_new);
1939
1940 /* Continue until we make progress */
1941 if (class_index_new >= class_index_old ||
1942 (threshold && comp_len_new >= threshold)) {
1943 zcomp_stream_put(zram->comps[prio], zstrm);
Imagine we hit this continue path. The right thing is probably to set
zstrm = NULL before the continue or it might be to set ret = -EINVAL.
1944 continue;
1945 }
1946
1947 /* Recompression was successful so break out */
1948 break;
1949 }
1950
1951 zram_slot_write_lock(zram, index);
1952 /* Compression error */
1953 if (ret) {
1954 zcomp_stream_put(zram->comps[prio], zstrm);
1955 return ret;
1956 }
1957
1958 /* Slot has been modified concurrently */
1959 if (!zram_test_flag(zram, index, ZRAM_PP_SLOT)) {
--> 1960 zcomp_stream_put(zram->comps[prio], zstrm);
^^^^^
Use after free
1961 return 0;
1962 }
1963
1964 /*
1965 * We did not try to recompress, e.g. when we have only one
1966 * secondary algorithm and the page is already recompressed
1967 * using that algorithm
1968 */
1969 if (!zstrm)
^^^^^^
Could be a non-NULL freed pointer.
1970 return 0;
regards,
dan carpenter
